[PATCH] D68488: [PATCH 03/27] [noalias] [IR] Introduce ptr_provenance for LoadInst/StoreInst

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

In D68488#2278602 <https://reviews.llvm.org/D68488#2278602>, @jeroen.dobbelaere wrote:

> I have 3 variants of this patch:
>
> - variant 0 : always reserve place for the ptr_provenance; Introduce NumOfOperandsDelta to get the correct number of operands
>   - extra space overhead for load/store instructions;
>   - extra instruction overhead for ALL instructions ('getNumOperand()');
>   - low overhead when adding/removing ptr_provenance
>   - NOTE: this is the current provided patch
> - variant 1 : always reserve place for the ptr_provenance; Move arguments around when adding/removing ptr_provenance
>   - extra space overhead for load/store instructions;
>   - NO impact on other instructions;
>   - slightly more overhead when accessing operands of load/store instructions;
>   - medium overhead when adding/removing ptr_provenance
> - variant 2 : Have a LoadInst, LoadWithProvInst and StoreInst, StoreWithProvInst. Only the 'WithProv' variant reserves place for optional argument. When the ptr_provenance is added, it can involve  creating a Load/StoreWithProv based on the original Load/Store. Adding/removing the ptr_provenance argument involves moving arguments around.
>   - extra space only needed when we know we will need it.
>   - NO impact on other instructions;
>   - slightly more overhead when accessing operands of load/store instructions;
>   - medium to high overhead when adding ptr_provenance. medium when removing it.
>   - higher risk in keeping things correct in optimization: adding the ptr_provenance can require recreating the Load/Store instruction. This might be difficult if the optimization pass is tracking pointers to the original Load/Store instruction.
>   - main benefit: almost no impact from this change if the feature is not used (aka -fno-full-restrict)
>
> Any preference in the way forward ? variant 1 is the easy one, as it is just a drop-in. Variant 2 is at the moment less-stable as it requires changes in all optimization passes that need to track the ptr_provenance.

I think `variant 1` is the obvious winner.
Though while i'm not familiar with the code, i'm not sure why you'd need to move args around - can't `ptr_provenance` be added as a new last 'argument'?


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

https://reviews.llvm.org/D68488