[PATCH] D102215: [SelectionDAG] Propagate scoped AA metadata when lowering mem intrinsics.

[Jeroen Dobbelaere via Phabricator via llvm-commits]

jeroen.dobbelaere added a comment.

I added some remarks.



================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp:6361
             DAG.getMemBasePlusOffset(Dst, TypeSize::Fixed(DstOff), dl),
-            DstPtrInfo.getWithOffset(DstOff), Alignment, MMOFlags);
+            DstPtrInfo.getWithOffset(DstOff), Alignment, MMOFlags, AAInfo);
         OutChains.push_back(Store);
----------------
IMHO, the expansion of the mem intrinsic should be responsible for only using the metadata that makes sense for it ?
(If not, we should have an assertion to check that the AAInfo is sane).

`!alias.scope` and `!noalias` make sense, with the meaning: this memory instruction might alias with '!alias.scope` scopes, and will never alias with `!noalias` scopes.
 `!tbaa` does not make sense (I think), but `!tbaa.struct` does. The expansion might make use of it, or leave away.



================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:5749
+// metadata only.
+static inline AAMDNodes prepareAAInfoForMemTransferInst(const CallInst &I) {
+  AAMDNodes AAInfo;
----------------
See above: the expansion itself should be responsible for taking the elements that it gets.
At this level, we might check that the tbaa is sane, but IMHO, that should then only check that  `!tbaa` is `null`.


================
Comment at: llvm/test/CodeGen/AArch64/memcpy-scoped-aa.ll:79
+; MIR-NEXT: STRXui %2, %0, 1 :: (store 8 into %ir.p0 + 8, align 4, !alias.scope ![[SET0]], !noalias ![[SET1]])
+; MIR-NEXT: STRXui %2, %0, 0 :: (store 8 into %ir.p0, align 4, !alias.scope ![[SET0]], !noalias ![[SET1]])
+define i32 @test_memset(i32* nocapture %p, i32* nocapture readonly %q) {
----------------
Can we also have the check for the other instructions (with scope info) at MIR level ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102215