[VectorCombine] loosen alignment constraint for load transform

[spatel]

Summary

As discussed in D93229, we only need a minimal alignment constraint when querying whether a hypothetical vector load is safe. We still pass/use the potentially stronger alignment attribute when checking costs and creating the new load.

There's already a test that changes with the minimum code change, so splitting this off as a preliminary proposal independent of any gep/offset enhancements.

[lebedev.ri]

LGTM, thanksThi, this makes sense to me.
float variant takes too long to proof, but i8 works fine:

----------------------------------------
define <4 x i8> @src(* dereferenceable(4) align(1) %p) {
%0:
  %s = load i8, * dereferenceable(4) align(1) %p, align 4
  %r = insertelement <4 x i8> undef, i8 %s, i32 0
  ret <4 x i8> %r
}
=>
define <4 x i8> @tgt(* dereferenceable(4) align(1) %p) {
%0:
  %1 = bitcast * dereferenceable(4) align(1) %p to *
  %2 = load <4 x i8>, * %1, align 4
  %r = shufflevector <4 x i8> %2, <4 x i8> undef, 0, 4294967295, 4294967295, 4294967295
  ret <4 x i8> %r
}
Transformation seems to be correct!

llvm/lib/Transforms/Vectorize/VectorCombine.cpp

141–142

This probably warrants a comment why we pass this alignment (that we only care about dereferenceability)

llvm/test/Transforms/VectorCombine/X86/load.ll

407

They are not really in conflict, just the first one being overly pessimistic.
Since we know what the alignment must be on all execution paths,
the alignment specified on the argument could be enhanced by knowledge backpropagation:
https://godbolt.org/z/dvqEEb

[spatel]

llvm/test/Transforms/VectorCombine/X86/load.ll

407

Ah, thanks for the explanation. I was thinking of a gep case where the alignment could be over-specified, but that will be UB based on:
http://llvm.org/docs/LangRef.html#load-instruction
"Overestimating the alignment results in undefined behavior."

[lebedev.ri]

llvm/test/Transforms/VectorCombine/X86/load.ll

409

align 1 here doesn't actually say that the alignment is/will be exactly 1, it specifies the lower bound,
it is perfectly fine for the actual pointer to be 1024-byte aligned instead.

llvm/lib/Transforms/Vectorize/VectorCombine.cpp

bool VectorCombine::vectorizeLoadInsert(Instruction &I) {
 
   Type *ScalarTy = Scalar->getType();
   uint64_t ScalarSize = ScalarTy->getPrimitiveSizeInBits();
   unsigned MinVectorSize = TTI.getMinVectorRegisterBitWidth();
   if (!ScalarSize || !MinVectorSize || MinVectorSize % ScalarSize != 0)
     return false;
 
   // Check safety of replacing the scalar load with a larger vector load.
+  // We use minimal alignment (maximum flexibility) because we only care about
+  // the dereferenceable region. When calculating cost and creating a new op,
+  // we may use a larger value based on alignment attributes.
   unsigned MinVecNumElts = MinVectorSize / ScalarSize;
   auto *MinVecTy = VectorType::get(ScalarTy, MinVecNumElts, false);
-  Align Alignment = Load->getAlign();
-  if (!isSafeToLoadUnconditionally(SrcPtr, MinVecTy, Alignment, DL, Load, &DT))
+  if (!isSafeToLoadUnconditionally(SrcPtr, MinVecTy, Align(1), DL, Load, &DT))
     return false;
 
   // Original pattern: insertelt undef, load [free casts of] PtrOp, 0
+  Align Alignment = Load->getAlign();
   Type *LoadTy = Load->getType();
   int OldCost = TTI.getMemoryOpCost(Instruction::Load, LoadTy, Alignment, AS);
   APInt DemandedElts = APInt::getOneBitSet(MinVecNumElts, 0);
   OldCost += TTI.getScalarizationOverhead(MinVecTy, DemandedElts,
                                           /* Insert */ true, HasExtract);
 
   // New pattern: load VecPtr
   int NewCost = TTI.getMemoryOpCost(Instruction::Load, MinVecTy, Alignment, AS);

llvm/test/Transforms/VectorCombine/X86/load.ll

 ; CHECK-NEXT:    [[R:%.*]] = insertelement <4 x float> undef, float [[S]], i32 0
 ; CHECK-NEXT:    ret <4 x float> [[R]]
 ;
   %s = load volatile float, float* %p, align 4
   %r = insertelement <4 x float> undef, float %s, i32 0
   ret <4 x float> %r
 }
 
-; Negative test? - pointer is not as aligned as load.
+; Pointer is not as aligned as load, but that's ok.
+; The new load uses the larger alignment value.
 
 define <4 x float> @load_f32_insert_v4f32_align(float* align 1 dereferenceable(16) %p) {
 ; CHECK-LABEL: @load_f32_insert_v4f32_align(
-; CHECK-NEXT:    [[S:%.*]] = load float, float* [[P:%.*]], align 4
-; CHECK-NEXT:    [[R:%.*]] = insertelement <4 x float> undef, float [[S]], i32 0
+; CHECK-NEXT:    [[TMP1:%.*]] = bitcast float* [[P:%.*]] to <4 x float>*
+; CHECK-NEXT:    [[TMP2:%.*]] = load <4 x float>, <4 x float>* [[TMP1]], align 4
+; CHECK-NEXT:    [[R:%.*]] = shufflevector <4 x float> [[TMP2]], <4 x float> undef, <4 x i32> <i32 0, i32 undef, i32 undef, i32 undef>
 ; CHECK-NEXT:    ret <4 x float> [[R]]
 ;
   %s = load float, float* %p, align 4
   %r = insertelement <4 x float> undef, float %s, i32 0
   ret <4 x float> %r
 }
 
 ; Negative test - not enough bytes.