[PATCH] D81960: [Matrix] Use alignment info when lowering loads/stores.

[John McCall via Phabricator via llvm-commits]

rjmccall added a comment.

Otherwise LGTM



================
Comment at: llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp:779
+    if (Idx == 0)
+      return InitialAlign;
+
----------------
This fast path should be checked before getting ElementAlign or ElementSize.


================
Comment at: llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp:785
+    }
+    return commonAlignment(InitialAlign, ElementAlign);
+  }
----------------
This should be `commonAlignment(InitialAlign, ElementSize / 8)`.  If you have an array of 8-byte objects, and the start is 8-byte-aligned, all the elements are 8-byte-aligned regardless of the natural alignment of the type.

Also, the right default assumption should always be that the DataLayout's built-in concept of type alignment is bad and wrong.  Once there's an explicit alignment given in the IR, there is no reason that the DataLayout alignment should ever play a role in the calculation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81960