[PATCH] D125485: [ArgPromotion] Unify byval promotion with non-byval

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

In D125485#3510990 <https://reviews.llvm.org/D125485#3510990>, @psamolysov wrote:

> @nikic Thank you for detailed description of the idea. I'm not sure that I get the idea correctly, if to analyse your comment by points:
>
> 1. I believe this is implemented in the patch.

Not quite: We should set `IsStoresAllowed == true` for all byval+align arguments (independently of "densely packed" and "padding access" predicates), and stores shouldn't be unconditionally allowed, but go through HandleLoad (well, a slight generalization that works on both load&store, but basically does the same thing).

> 2. If stores are allowed (IsStoresAllowed in the code), the findArgParts function should disable the AAR.canInstructionRangeModRef and AAR.canBasicBlockModify(*TranspBB, Loc) checks because the pointers may be invalidated.

Correct.

> 3. This makes the most difficult thing for me. As I get your point, all accesses (load/stores) to the arguments must be replaced with accesses to new allocas with the corresponding types, so instead of just eliminate loads we should for every promotable pointer argument (regardless whether it is byval or not) create an alloca, add a store of the new value argument into this alloca, keep all the allowed stores and loads (they will work with this alloca) and then call the 'promoteMemoryToRegister' function to do a part of mem2reg's work.

Yeah, exactly. Basically do what byval promotion currently does, just based on ArgParts and not byval structure, and add a mem2reg call at the end.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125485/new/

https://reviews.llvm.org/D125485