[VectorCombine] Preserve scoped alias metadata

[knwng]

Right now if a load op is scalarized, the !alias.scope and !noalias metadata are dropped. This PR is to keep them if exist.

[llvmbot]

@llvm/pr-subscribers-vectorizers

@llvm/pr-subscribers-llvm-transforms

Author: Kyle Wang (knwng)

Changes

Right now if a load op is scalarized, the !alias.scope and !noalias metadata are dropped. This PR is to keep them if exist.

---

Full diff: https://github.com/llvm/llvm-project/pull/153714.diff

1 Files Affected:

• (modified) llvm/lib/Transforms/Vectorize/VectorCombine.cpp (+12)

diff --git a/llvm/lib/Transforms/Vectorize/VectorCombine.cpp b/llvm/lib/Transforms/Vectorize/VectorCombine.cpp
index 4a681cbdab8ca..587889873a778 100644
--- a/llvm/lib/Transforms/Vectorize/VectorCombine.cpp
+++ b/llvm/lib/Transforms/Vectorize/VectorCombine.cpp
@@ -1811,6 +1811,10 @@ bool VectorCombine::scalarizeLoadExtract(Instruction &I) {
   // erased in the correct order.
   Worklist.push(LI);
 
+  LLVMContext &ctx = LI->getContext();
+  unsigned aliasScopeKind = ctx.getMDKindID("alias.scope");
+  unsigned noAliasKind = ctx.getMDKindID("noalias");
+
   // Replace extracts with narrow scalar loads.
   for (User *U : LI->users()) {
     auto *EI = cast<ExtractElementInst>(U);
@@ -1831,6 +1835,14 @@ bool VectorCombine::scalarizeLoadExtract(Instruction &I) {
         LI->getAlign(), VecTy->getElementType(), Idx, *DL);
     NewLoad->setAlignment(ScalarOpAlignment);
 
+    if (MDNode *aliasScope = LI->getMetadata(aliasScopeKind)) {
+      NewLoad->setMetadata(aliasScopeKind, aliasScope);
+    }
+
+    if (MDNode *noAlias = LI->getMetadata(noAliasKind)) {
+      NewLoad->setMetadata(noAliasKind, noAlias);
+    }
+
     replaceValue(*EI, *NewLoad);
   }
 

[bcahoon]

Hi @knwng, thanks! I haven't look at the patch in any detail. It does need a lit test though.

[nikic]

If possible, it would be better to use getAAMetadata() and adjustForAccess() here.

[knwng]

Hi @nikic, thank you for the advice. I know it's a more thoughtful solution. But I'm not familiar with AA in LLVM. Can you guide me on how to use adjustForAccess(), like how to correctly calculate offset and type? Thanks!

[nikic]

The type is VecTy->getElementType(). The offset would be Idx * the type size like here:

llvm-project/llvm/lib/Transforms/Vectorize/VectorCombine.cpp

Lines 1652 to 1654 in adce747

	if (auto *C = dyn_cast<ConstantInt>(Idx))
	return commonAlignment(VectorAlignment,
	C->getZExtValue() * DL.getTypeStoreSize(ScalarType));

The problem is if Idx is a variable. In that case the offset is unknown. So I would make the whole logic conditional on the dyn_cast<ConstantInt>(Idx) case.

[RKSimon]

Did you find this by inspection or was there a real world case that we can create test from please?

[frederik-h]

Hi @knwng, thanks! I haven't look at the patch in any detail. It does need a lit test though.

I have written a test that reproduces the issue: 13fa0d2.
@knwng I hope this helps as a starting point.

[knwng]

Did you find this by inspection or was there a real world case that we can create test from please?

It's found in a real world case. @frederik-h 's test is extracted from it.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[nikic]

Please do not reformat unrelated code (you can use git-clang-format to only format changed lines).

[knwng]

Ah sorry. Fixed

[knwng]

Hi @nikic, is the CI failure a runner issue?

[nikic]

LGTM

[frederik-h]

Hi @nikic, is the CI failure a runner issue?

Looks like it is. If you retrigger the run, e.g. after you resolve the conflict, it will probably work.