[PATCH] D82565: Fix invalid alignment in DAGCombiner::isLegalNarrowLdSt

[Guillaume Chatelet via Phabricator via llvm-commits]

gchatelet created this revision.
gchatelet added a reviewer: samparker.
Herald added subscribers: llvm-commits, hiraditya.
Herald added a project: LLVM.
gchatelet marked an inline comment as done.
gchatelet added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:4879
+  if (ShAmt) {
+    assert(ShAmt % 8 == 0 && "ShAmt is byte offset");
+    const unsigned ByteShAmt = ShAmt / 8;
----------------
`assert` here is just to refresh what is established L.4861
It helps understand why `ShAmt / 8` is valid.


`ShAmt / 8` can be a non power of two, this can lead to an invalid alignment.
context: https://reviews.llvm.org/D41350#inline-749165


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D82565

Files:
  llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
  llvm/test/CodeGen/ARM/and-load-combine.ll


Index: llvm/test/CodeGen/ARM/and-load-combine.ll
===================================================================
--- llvm/test/CodeGen/ARM/and-load-combine.ll
+++ llvm/test/CodeGen/ARM/and-load-combine.ll
@@ -1433,12 +1433,9 @@
 ;
 ; THUMB1-LABEL: test23:
 ; THUMB1:       @ %bb.0:
-; THUMB1-NEXT:    ldrb r1, [r0, #3]
-; THUMB1-NEXT:    ldrb r0, [r0, #4]
-; THUMB1-NEXT:    lsls r0, r0, #8
-; THUMB1-NEXT:    adds r1, r0, r1
-; THUMB1-NEXT:    lsls r0, r1, #24
-; THUMB1-NEXT:    lsrs r1, r1, #8
+; THUMB1-NEXT:    ldrb r1, [r0, #4]
+; THUMB1-NEXT:    ldrb r0, [r0, #3]
+; THUMB1-NEXT:    lsls r0, r0, #24
 ; THUMB1-NEXT:    bx lr
 ;
 ; THUMB2-LABEL: test23:
@@ -1498,13 +1495,15 @@
 ;
 ; THUMB1-LABEL: test25:
 ; THUMB1:       @ %bb.0:
-; THUMB1-NEXT:    ldrb r1, [r0, #5]
-; THUMB1-NEXT:    ldrb r0, [r0, #6]
-; THUMB1-NEXT:    lsls r0, r0, #8
-; THUMB1-NEXT:    adds r0, r0, r1
-; THUMB1-NEXT:    lsls r1, r0, #8
+; THUMB1-NEXT:    ldr r0, [r0, #4]
+; THUMB1-NEXT:    ldr r1, .LCPI37_0
+; THUMB1-NEXT:    ands r1, r0
 ; THUMB1-NEXT:    movs r0, #0
 ; THUMB1-NEXT:    bx lr
+; THUMB1-NEXT:    .p2align 2
+; THUMB1-NEXT:  @ %bb.1:
+; THUMB1-NEXT:  .LCPI37_0:
+; THUMB1-NEXT:    .long 16776960 @ 0xffff00
 ;
 ; THUMB2-LABEL: test25:
 ; THUMB2:       @ %bb.0:
Index: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
===================================================================
--- llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -4875,11 +4875,16 @@
     return false;
 
   // Ensure that this isn't going to produce an unsupported memory access.
-  if (ShAmt &&
-      !TLI.allowsMemoryAccess(*DAG.getContext(), DAG.getDataLayout(), MemVT,
-                              LDST->getAddressSpace(), ShAmt / 8,
-                              LDST->getMemOperand()->getFlags()))
-    return false;
+  if (ShAmt) {
+    assert(ShAmt % 8 == 0 && "ShAmt is byte offset");
+    const unsigned ByteShAmt = ShAmt / 8;
+    const Align LDSTAlign = LDST->getAlign();
+    const Align NarrowAlign = commonAlignment(LDSTAlign, ByteShAmt);
+    if (!TLI.allowsMemoryAccess(*DAG.getContext(), DAG.getDataLayout(), MemVT,
+                                LDST->getAddressSpace(), NarrowAlign.value(),
+                                LDST->getMemOperand()->getFlags()))
+      return false;
+  }
 
   // It's not possible to generate a constant of extended or untyped type.
   EVT PtrType = LDST->getBasePtr().getValueType();


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D82565.273380.patch
Type: text/x-patch
Size: 2468 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200625/fba6a02c/attachment.bin>