[PATCH] D104268: [ptr_provenance] Introduce optional ptr_provenance operand to load/store

[Jeroen Dobbelaere via Phabricator via llvm-commits]

jeroen.dobbelaere added a comment.

In D104268#3107611 <https://reviews.llvm.org/D104268#3107611>, @asbirlea wrote:

> - Discussion on why is cloning a load/store setting the ptr_provenance to unknown_provenance? When cloning a load/store instruction, the cloned instruction needs to have the same number of operands. Otherwise crashes were observed in some passes (example given: LoopVectorization). Why can't the number of operands remain the same, but set the ptr_provenance to the (otherwise default) ptr_operand?
>
> @jeroen.dobbelaere: Could you describe using unknown for the cloning question for future reference?

One of the goals when introducing the optional parameter, was to reduce the impact on existing code/optimizations as much as possible. When cloning the instruction there are 5 possible cases to be considered:

1. omit (=do not clone) the ptr_provenance operand. This resulted in problems, as now, the number of operands between the original and the cloned instruction can differ.
2. clone the ptr_provenance operand when available. This resulted in dominance violations for some optimization passes that are not aware of the provenance operand.
3. When available, replace the ptr_provenance with the 'pointer' operand in the clone. This results in an extra use of the pointer operand, potentially resulting in worse code for  optimizations depending on 'hasOneUse()'.
4. When available, replace the ptr_provenance with `undef` in the clone. (Special value for the original Full Restrict)
5. When available, replace the ptr_provenance with `unknown_provenance` in the clone.

In the original full restrict patches, (4) is done, as this (together with not copying !noalias metadata) resulted in the safest approach. A `undef` ptr_provenance is treated there as: look at the pointer operand for the provenance, aka similar to omitting the ptr_provenance, without actually doing it (1), and also similar to (3), but without introducing an extra 'use' on the pointer operand.

When preparing this part of the infrastructure, I wanted to take into account a more generic usage of the ptr_provenance infrastructure. For that, a special constant `unknown_provenance` is introduced. This indicates that, for the cloned access, we don't know the ptr_provenance (and associated noalias annotations). We can also not depend on the pointer operand for the provenance, as the pointer value can have a different provenance (!). Only when the optimization pass that is introducing the clone is aware of the ptr_provenance operand, the right decision can be taken, which can result in better code. So, (5) is the safe approach with minimal impact on existing code.


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

https://reviews.llvm.org/D104268