[PATCH] D152790: [ARM] Fix codegen of unaligned volatile load/store of i64
Maurice Heumann via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Jun 15 03:04:38 PDT 2023
momo5502 updated this revision to Diff 531670.
momo5502 added a comment.
In D152790#4419917 <https://reviews.llvm.org/D152790#4419917>, @efriedma wrote:
> I have no idea why the ARMLoadStoreOptimizer code was written that way; maybe fix that too, for the sake of clarity.
I adjusted it here to Align(4).
I also tried to adjust the ARMLoadStoreOptimizer and it turns out that getABITypeAlign of i64 is 8 and not 4,. That seems wrong in case of aligning loads. Therefore your comment is valid and it should be fixed there too. However, that breaks quite a bunch of tests and I feel this exceeds the scope of this change. I recommend creating an issue for that.
@dmgreen, concerning `allowsUnalignedMem`, I'm not sure how to proceed with that. Only applying the alignment check form this change when strict-alignment is enabled causes the instructions in this tests to be lowered to 16 one-byte loads and stores. However, 2 four-byte loads seem perfectly fine to me, at least on the system where I encountered this issue. To me, not taking `allowsUnalignedMem` into consideration seems fine. What do you think?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D152790/new/
https://reviews.llvm.org/D152790
Files:
llvm/lib/Target/ARM/ARMISelLowering.cpp
llvm/test/CodeGen/ARM/i64_volatile_load_store.ll
Index: llvm/test/CodeGen/ARM/i64_volatile_load_store.ll
===================================================================
--- llvm/test/CodeGen/ARM/i64_volatile_load_store.ll
+++ llvm/test/CodeGen/ARM/i64_volatile_load_store.ll
@@ -5,6 +5,9 @@
@x = common dso_local global i64 0, align 8
@y = common dso_local global i64 0, align 8
+ at x_unaligned = common dso_local global i64 0, align 1
+ at y_unaligned = common dso_local global i64 0, align 1
+
define void @test() {
entry:
; CHECK-LABEL: test:
@@ -29,6 +32,34 @@
ret void
}
+define void @test_unaligned() {
+entry:
+; CHECK-LABEL: test_unaligned:
+; CHECK-ARMV5TE: ldr [[ADDR0:r[0-9]+]]
+; CHECK-ARMV5TE-NEXT: ldr [[ADDR1:r[0-9]+]]
+; CHECK-ARMV5TE-NEXT: ldr [[R1:r[0-9]+]], [[[ADDR0]]]
+; CHECK-ARMV5TE-NEXT: ldr [[R0:r[0-9]+]], [[[ADDR0]], #4]
+; CHECK-ARMV5TE-NEXT: str [[R0]], [[[ADDR1]], #4]
+; CHECK-ARMV5TE-NEXT: str [[R1]], [[[ADDR1]]]
+; CHECK-T2: movw [[ADDR0:r[0-9]+]], :lower16:x_unaligned
+; CHECK-T2-NEXT: movw [[ADDR1:r[0-9]+]], :lower16:y_unaligned
+; CHECK-T2-NEXT: movt [[ADDR0]], :upper16:x_unaligned
+; CHECK-T2-NEXT: movt [[ADDR1]], :upper16:y_unaligned
+; CHECK-T2-NEXT: ldr [[R1]], [[[ADDR0]]]
+; CHECK-T2-NEXT: ldr [[R0]], [[[ADDR0]], #4]
+; CHECK-T2-NEXT: str [[R0]], [[[ADDR1]], #4]
+; CHECK-T2-NEXT: str [[R1]], [[[ADDR1]]]
+; CHECK-ARMV4T: ldr [[ADDR0:r[0-9]+]]
+; CHECK-ARMV4T-NEXT: ldr [[ADDR1:r[0-9]+]]
+; CHECK-ARMV4T-NEXT: ldr [[R1:r[0-9]+]], [[[ADDR0]]]
+; CHECK-ARMV4T-NEXT: ldr [[R0:r[0-9]+]], [[[ADDR0]], #4]
+; CHECK-ARMV4T-NEXT: str [[R0]], [[[ADDR1]], #4]
+; CHECK-ARMV4T-NEXT: str [[R1]], [[[ADDR1]]]
+ %0 = load volatile i64, ptr @x_unaligned, align 1
+ store volatile i64 %0, ptr @y_unaligned, align 1
+ ret void
+}
+
define void @test_offset() {
entry:
; CHECK-LABEL: test_offset:
Index: llvm/lib/Target/ARM/ARMISelLowering.cpp
===================================================================
--- llvm/lib/Target/ARM/ARMISelLowering.cpp
+++ llvm/lib/Target/ARM/ARMISelLowering.cpp
@@ -10080,7 +10080,8 @@
assert(LD->isUnindexed() && "Loads should be unindexed at this point.");
if (MemVT == MVT::i64 && Subtarget->hasV5TEOps() &&
- !Subtarget->isThumb1Only() && LD->isVolatile()) {
+ !Subtarget->isThumb1Only() && LD->isVolatile() &&
+ LD->getAlign() >= Align(Subtarget->hasV6Ops() ? 4 : 8)) {
SDLoc dl(N);
SDValue Result = DAG.getMemIntrinsicNode(
ARMISD::LDRD, dl, DAG.getVTList({MVT::i32, MVT::i32, MVT::Other}),
@@ -10137,7 +10138,8 @@
assert(ST->isUnindexed() && "Stores should be unindexed at this point.");
if (MemVT == MVT::i64 && Subtarget->hasV5TEOps() &&
- !Subtarget->isThumb1Only() && ST->isVolatile()) {
+ !Subtarget->isThumb1Only() && ST->isVolatile() &&
+ ST->getAlign() >= Align(Subtarget->hasV6Ops() ? 4 : 8)) {
SDNode *N = Op.getNode();
SDLoc dl(N);
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D152790.531670.patch
Type: text/x-patch
Size: 2925 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230615/b173d200/attachment.bin>
More information about the llvm-commits
mailing list