[PATCH] D93397: [VectorCombine] loosen alignment constraint for load transform

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri accepted this revision.
lebedev.ri added a comment.
This revision is now accepted and ready to land.

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!



================
Comment at: llvm/lib/Transforms/Vectorize/VectorCombine.cpp:138-139
   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;
----------------
This probably warrants a comment why we pass this alignment (that we only care about dereferenceability)


================
Comment at: llvm/test/Transforms/VectorCombine/X86/load.ll:407
+; Pointer is not as aligned as load, but that's ok.
+; TODO: What does it mean when alignment attributes are in conflict?
 
----------------
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


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

https://reviews.llvm.org/D93397