[PATCH] D76944: [GVN] Fix VNCoercion/BasicAA for Scalable Vector.

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: llvm/lib/Analysis/BasicAliasAnalysis.cpp:543
+      Type *GEPIdxedTy = GTI.getIndexedType();
+      if (GEPIdxedTy->isVectorTy() && GEPIdxedTy->getVectorIsScalable()) {
+        GepHasConstantOffset = false;
----------------
Can you check DL.getTypeAllocSize(GEPIdxedTy).Scalable instead, since you're calling getTypeAllocSize anyway?

I don't think this is the right way to break out of this loop early; if the offset isn't recorded in "Decomposed", other code might assume it's zero?


================
Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:1880
     Type *EltTy = GTI.getIndexedType();
-    if (EltTy->isSized() && DL.getTypeAllocSize(EltTy) == 0)
+    if (EltTy->isSized() && !DL.getTypeAllocSize(EltTy).isNonZero())
       if (!isa<Constant>(*I) || !match(I->get(), m_Zero())) {
----------------
I think TypeSize also has isZero now, assuming that got merged.


================
Comment at: llvm/lib/Transforms/Utils/VNCoercion.cpp:21
   // If the loaded or stored value is an first class array or struct, don't try
   // to transform them.  We need to be able to bitcast to integer.
   if (LoadTy->isStructTy() || LoadTy->isArrayTy() || StoredTy->isStructTy() ||
----------------
Fix the comment here?

Can you refactor the whole `Ty->isStructTy() || LoadTy->isArrayTy() || (Ty->isVectorTy() && Ty->getVectorIsScalable())` thing into a helper function, so the overall property has a name?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76944