[flang][fir] always use memcpy for fir.box #1

ashermancinelli · 2024-10-24T23:58:00Z

@jeanPerier explained the importance of converting box loads and stores into memcpys instead of aggregate loads and stores, and I'll do my best to explain it here.

(godbolt link) Example comparing opt transformations on memcpys vs aggregate load/stores
- LLVM can more effectively reason about memcpys compared to aggregate load/stores.
- This came up when others were discussing array descriptors for assumed-rank arrays passed to bind(c) subroutines, with the implication that the array descriptors are known to have lower bounds of 1 and that they are not pointer/allocatable types.
(godbolt link) Clang also uses memcpys so we should probably follow them, assuming the clang developers are generatign what they know Opt will handle more effectively.
This currently may not help much without the nocapture attribute being propagated to function calls, but it looks like someone may do this soon (discourse link) or I can do this in a follow-up patch.

LLVM is not as effective at optimizing aggregate loads and stores as it is optimizing memcpy calls, so we always generate the latter for load and stores between fir.box objects.

Adds checks for allocas used for temp storage between a fir.ref<fir.box> and a fir.box.

jeanPerier

LGTM, thanks, you can move your PR to LLVM (use [flang][fir] as a PR title prefix there)

[fir] Always use memcpy for fir.load/store

58bc994

LLVM is not as effective at optimizing aggregate loads and stores as it is optimizing memcpy calls, so we always generate the latter for load and stores between fir.box objects.

ashermancinelli changed the title ~~[Fir]Fir box always memcpy~~ [Fir] always use memcpy for fir.box Oct 25, 2024

ashermancinelli added 4 commits October 25, 2024 08:53

[fir][test] Update box.fir tests

b8b403b

Adds checks for allocas used for temp storage between a fir.ref<fir.box> and a fir.box.

[tests] Update lit tests wrt memcpy change

0d276b6

update tests

1de3879

update tests (all passing now)

57965ad

jeanPerier approved these changes Oct 28, 2024

View reviewed changes

ashermancinelli changed the title ~~[Fir] always use memcpy for fir.box~~ [flang][fir] always use memcpy for fir.box Oct 28, 2024

ashermancinelli closed this Oct 28, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[flang][fir] always use memcpy for fir.box #1

[flang][fir] always use memcpy for fir.box #1

ashermancinelli commented Oct 24, 2024 •

edited

Loading

jeanPerier left a comment

[flang][fir] always use memcpy for fir.box #1

[flang][fir] always use memcpy for fir.box #1

Conversation

ashermancinelli commented Oct 24, 2024 • edited Loading

jeanPerier left a comment

Choose a reason for hiding this comment

ashermancinelli commented Oct 24, 2024 •

edited

Loading