[PATCH] D52310: Set correct MMO offset on scalarized load pieces

[David Greene via Phabricator via llvm-commits]

greened added inline comments.


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:19889
+  unsigned Offset = 0;
   for (unsigned i = 0; i < NumLoads; ++i) {
+    unsigned NewAlign = MinAlign(Ld->getAlignment(), Offset);
----------------
arsenm wrote:
> Why doesn't this use TLI::scalarizeVectorLoad?
I don't know.  Perhaps we can look at refactoring after fixing this bug.


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:19890
   for (unsigned i = 0; i < NumLoads; ++i) {
+    unsigned NewAlign = MinAlign(Ld->getAlignment(), Offset);
+
----------------
craig.topper wrote:
> I'm not sure if this is needed. getAlignment for loads defers to MachineMemOperand::getAlignment which calls MinAlign(getBaseAlignment(), Offset)
I was just following the example of similar code in the file.  If we think it's unnecessary, I guess that's ok but it seems good to me to make this kind of thing explicit, if for no other reason than to make others feel comfortable when they look at this code two years from now.


Repository:
  rL LLVM

https://reviews.llvm.org/D52310