[PATCH] D29446: [AArch64] Fix incorrect MachinePointerInfo in splitStoreSplat

[Geoff Berry via Phabricator via llvm-commits]

gberry added a comment.

Nice catch.  Coincidentally I'm currently working on a patch that fixes a different bug related to this same transform.

Would you consider adding test cases that cover the other caller of splitStoreSplat, namely splitStores when it sees a store of a splat vector?

It also might be worth considering whether the tests should check the scheduler dependency debug output, since otherwise this bug is only exposed if the scheduler happens to pick a bad schedule.

Also, this seems like it probably should be considered for the release branch.



================
Comment at: lib/Target/AArch64/AArch64ISelLowering.cpp:8926
   SDValue BasePtr = St.getBasePtr();
+  MachinePointerInfo PtrInfo = St.getPointerInfo();
   SDValue NewST1 =
----------------
This should probably be const MachinePointerInfo &PtrInfo


================
Comment at: test/CodeGen/AArch64/ldst-zero.ll:3
+
+; Tests to check that zero stores which are generates as STP xzr, xzr aren't
+; scheduled incorrectly due to incorrect alias information
----------------
typo "generates" -> "generated"


================
Comment at: test/CodeGen/AArch64/ldst-zero.ll:11
+define void @test1(%struct.tree_common* %t, i32 %code, i8* %type) {
+; CHECK-LABEL: test1
+; CHECK: stp xzr, xzr, [x0, #8]
----------------
All of the CHECK-LABELS should probably include a ':' at the end to make the matching tighter:
e.g. CHECK-LABEL: test1:


Repository:
  rL LLVM

https://reviews.llvm.org/D29446