[PATCH] D68488: [PATCH 05/38] [noalias] [IR] Introduce noalias_sidechannel for LoadInst/StoreInst

[Jeroen Dobbelaere via Phabricator via llvm-commits]

jeroen.dobbelaere marked an inline comment as done.
jeroen.dobbelaere added inline comments.


================
Comment at: llvm/lib/IR/Instructions.cpp:4149
+  //   that don't know how to handle it (Like MergeLoadStoreMotion shows)
+  // - safe alternative: keep the argument, but map it to undef.
+  if (hasNoaliasSideChannelOperand())
----------------
a.elovikov wrote:
> Why is it safe? Consider we have aliasing load and store, we clone them so load.clone and load.store now have undef in the sidechannels, so optimizations are free to choose the values for them that would mean noalias.
- `Undef` is maybe the wrong value and could indeed trigger problems in future.
- `null` is not safe, as that is used to indicate a converted load/store instruction, when it does not depend on a restrict pointer (still in combination with the !noalias metadata, which must be present and indicate what scopes are visible)..
- Copying the noalias_sidechannel from the original is also not safe. It might not be valid in the new context.
- Removing the noalias_sidechannel (as was done in some earlier version) also does not work: some passes rightfully expect the clone to have the same number of arguments as the original.



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

https://reviews.llvm.org/D68488