[LegalizeVectorTypes] Preserve original MemoryOperand and MemVT when widening fixed vector load to vp_load.

[topperc]

Previously we were building a new memoperand with the size of the widened VT. I haven't figured out why yet, but this was causing a failure in our downstream with non-power of 2 vectorization. Alias analysis to reschedule a 3 element vector load past 2 out of 3 scalar stores that overwrite what it was supposed to read.

I planning to keep debugging it, and maybe come up with a test case once I understand it.

[llvmbot]

@llvm/pr-subscribers-llvm-selectiondag

Author: Craig Topper (topperc)

Changes

Previously we were building a new memoperand with the size of the widened VT. I haven't figured out why yet, but this was causing a failure in our downstream with non-power of 2 vectorization. Alias analysis to reschedule a 3 element vector load past 2 out of 3 scalar stores that overwrite what it was supposed to read.

I planning to keep debugging it, and maybe come up with a test case once I understand it.

---

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

1 Files Affected:

• (modified) llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp (+3-4)

diff --git a/llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp b/llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
index 482f88e5c86de7..1c466ed0b77997 100644
--- a/llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
@@ -5857,11 +5857,10 @@ SDValue DAGTypeLegalizer::WidenVecRes_LOAD(SDNode *N) {
     SDValue Mask = DAG.getAllOnesConstant(DL, WideMaskVT);
     SDValue EVL = DAG.getElementCount(DL, TLI.getVPExplicitVectorLengthTy(),
                                       LdVT.getVectorElementCount());
-    const auto *MMO = LD->getMemOperand();
     SDValue NewLoad =
-        DAG.getLoadVP(WideVT, DL, LD->getChain(), LD->getBasePtr(), Mask, EVL,
-                      MMO->getPointerInfo(), MMO->getAlign(), MMO->getFlags(),
-                      MMO->getAAInfo());
+        DAG.getLoadVP(LD->getAddressingMode(), ISD::NON_EXTLOAD, WideVT, DL,
+                      LD->getChain(), LD->getBasePtr(), LD->getOffset(), Mask,
+                      EVL, LD->getMemoryVT(), LD->getMemOperand());
 
     // Modified the chain - switch anything that used the old chain to use
     // the new one.

[topperc]

The incorrect size caused us to hit this code in BasicAA.

  // If the size of one access is larger than the entire object on the other     
  // side, then we know such behavior is undefined and can assume no alias.      
  bool NullIsValidLocation = NullPointerIsDefined(&F);                           
  if ((isObjectSmallerThan(                                                      
          O2, getMinimalExtentFrom(*V1, V1Size, DL, NullIsValidLocation), DL,    
          TLI, NullIsValidLocation)) ||                                          
      (isObjectSmallerThan(                                                      
          O1, getMinimalExtentFrom(*V2, V2Size, DL, NullIsValidLocation), DL,    
          TLI, NullIsValidLocation)))                                            
    return AliasResult::NoAlias;

[lukel97]

LGTM, I just quickly checked to see what we do currently and we're increasing the MMO size at least for fixed vectors

t5: v3i64,ch = load<(load (s192) from %ir.p, align 32)> t0, t2, undef:i64

t19: v4i64,ch = vp_load<(load (s256) from %ir.p)> t0, t2, undef:i64, t17, Constant:i64<3>

[topperc]

LGTM, I just quickly checked to see what we do currently and we're increasing the MMO size at least for fixed vectors

t5: v3i64,ch = load<(load (s192) from %ir.p, align 32)> t0, t2, undef:i64

t19: v4i64,ch = vp_load<(load (s256) from %ir.p)> t0, t2, undef:i64, t17, Constant:i64<3>

I guess should have mentioned we already do the right thing when widening fixed vector stores.

[frasercrmck]

LGTM