[llvm] r311649 - [ARM, Thumb1] Prevent ARMTargetLowering::isLegalAddressingMode from accepting illegal modes

Evgeny Astigeevich via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 24 03:00:26 PDT 2017


Author: eastig
Date: Thu Aug 24 03:00:25 2017
New Revision: 311649

URL: http://llvm.org/viewvc/llvm-project?rev=311649&view=rev
Log:
[ARM, Thumb1] Prevent ARMTargetLowering::isLegalAddressingMode from accepting illegal modes

ARMTargetLowering::isLegalAddressingMode can accept illegal addressing modes
for the Thumb1 target. This causes generation of redundant code and affects
performance.

This fixes PR34106: https://bugs.llvm.org/show_bug.cgi?id=34106

Differential Revision: https://reviews.llvm.org/D36467

Added:
    llvm/trunk/test/CodeGen/Thumb/addr-modes.ll
    llvm/trunk/test/Transforms/LoopStrengthReduce/illegal-addr-modes.ll
Modified:
    llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp
    llvm/trunk/lib/Target/ARM/ARMISelLowering.h

Modified: llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp?rev=311649&r1=311648&r2=311649&view=diff
==============================================================================
--- llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp (original)
+++ llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp Thu Aug 24 03:00:25 2017
@@ -12383,6 +12383,21 @@ bool ARMTargetLowering::isLegalT2ScaledA
   }
 }
 
+bool ARMTargetLowering::isLegalT1ScaledAddressingMode(const AddrMode &AM,
+                                                      EVT VT) const {
+  const int Scale = AM.Scale;
+
+  // Negative scales are not supported in Thumb1.
+  if (Scale < 0)
+    return false;
+
+  // Thumb1 addressing modes do not support register scaling excepting the
+  // following cases:
+  // 1. Scale == 1 means no scaling.
+  // 2. Scale == 2 this can be lowered to r + r if there is no base register.
+  return (Scale == 1) || (!AM.HasBaseReg && Scale == 2);
+}
+
 /// isLegalAddressingMode - Return true if the addressing mode represented
 /// by AM is legal for this target, for a load/store of the specified type.
 bool ARMTargetLowering::isLegalAddressingMode(const DataLayout &DL,
@@ -12399,10 +12414,6 @@ bool ARMTargetLowering::isLegalAddressin
   switch (AM.Scale) {
   case 0:  // no scale reg, must be "r+i" or "r", or "i".
     break;
-  case 1:
-    if (Subtarget->isThumb1Only())
-      return false;
-    LLVM_FALLTHROUGH;
   default:
     // ARM doesn't support any R+R*scale+imm addr modes.
     if (AM.BaseOffs)
@@ -12411,6 +12422,9 @@ bool ARMTargetLowering::isLegalAddressin
     if (!VT.isSimple())
       return false;
 
+    if (Subtarget->isThumb1Only())
+      return isLegalT1ScaledAddressingMode(AM, VT);
+
     if (Subtarget->isThumb2())
       return isLegalT2ScaledAddressingMode(AM, VT);
 

Modified: llvm/trunk/lib/Target/ARM/ARMISelLowering.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMISelLowering.h?rev=311649&r1=311648&r2=311649&view=diff
==============================================================================
--- llvm/trunk/lib/Target/ARM/ARMISelLowering.h (original)
+++ llvm/trunk/lib/Target/ARM/ARMISelLowering.h Thu Aug 24 03:00:25 2017
@@ -329,6 +329,10 @@ class InstrItineraryData;
 
     bool isLegalT2ScaledAddressingMode(const AddrMode &AM, EVT VT) const;
 
+    /// \brief Returns true if the addresing mode representing by AM is legal
+    /// for the Thumb1 target, for a load/store of the specified type.
+    bool isLegalT1ScaledAddressingMode(const AddrMode &AM, EVT VT) const;
+
     /// isLegalICmpImmediate - Return true if the specified immediate is legal
     /// icmp immediate, that is the target has icmp instructions which can
     /// compare a register against the immediate without having to materialize

Added: llvm/trunk/test/CodeGen/Thumb/addr-modes.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/Thumb/addr-modes.ll?rev=311649&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/Thumb/addr-modes.ll (added)
+++ llvm/trunk/test/CodeGen/Thumb/addr-modes.ll Thu Aug 24 03:00:25 2017
@@ -0,0 +1,45 @@
+; REQUIRES: asserts
+; RUN: llc < %s -debug-only=codegenprepare -o /dev/null 2>&1 | FileCheck %s
+
+; These are regression tests for
+;  https://bugs.llvm.org/show_bug.cgi?id=34106
+;    "ARMTargetLowering::isLegalAddressingMode can accept incorrect
+;    addressing modes for Thumb1 target"
+;
+; The Thumb1 target addressing modes don't support scaling.
+; It supports: r1 + r2, where r1 and r2 can be the same register.
+
+target datalayout = "e-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64"
+target triple = "thumbv6m-arm-none-eabi"
+
+; Test case 01: %n is scaled by 4 (size of i32).
+; Expected: GEP cannot be folded into LOAD.
+; CHECK: local addrmode: [Base:%arrayidx]
+define i32 @load01(i32* %p, i32 %n) nounwind {
+entry:
+  %arrayidx = getelementptr inbounds i32, i32* %p, i32 %n
+  %0 = load i32, i32* %arrayidx, align 4
+  ret i32 %0
+}
+
+; Test case 02: No scale of %n is needed because the size of i8 is 1.
+; Expected: GEP can be folded into LOAD.
+; CHECK: local addrmode: [Base:%p + 1*%n]
+define i8 @load02(i8* %p, i32 %n) nounwind {
+entry:
+  %arrayidx = getelementptr inbounds i8, i8* %p, i32 %n
+  %0 = load i8, i8* %arrayidx
+  ret i8 %0
+}
+
+; Test case 03: 2*%x can be represented as %x + %x.
+; Expected: GEP can be folded into LOAD.
+; CHECK: local addrmode: [2*%x]
+define i32 @load03(i32 %x) nounwind {
+entry:
+  %mul = shl nsw i32 %x, 1
+  %0 = inttoptr i32 %mul to i32*
+  %1 = load i32, i32* %0, align 4
+  ret i32 %1
+}
+

Added: llvm/trunk/test/Transforms/LoopStrengthReduce/illegal-addr-modes.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/LoopStrengthReduce/illegal-addr-modes.ll?rev=311649&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/LoopStrengthReduce/illegal-addr-modes.ll (added)
+++ llvm/trunk/test/Transforms/LoopStrengthReduce/illegal-addr-modes.ll Thu Aug 24 03:00:25 2017
@@ -0,0 +1,122 @@
+; RUN: opt < %s -loop-reduce -S | FileCheck %s
+
+target datalayout = "e-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64"
+target triple = "thumbv6m-arm-none-eabi"
+
+; These are regression tests for
+;  https://bugs.llvm.org/show_bug.cgi?id=34106
+;    "ARMTargetLowering::isLegalAddressingMode can accept incorrect
+;    addressing modes for Thumb1 target"
+;  https://reviews.llvm.org/D34583
+;    "[LSR] Narrow search space by filtering non-optimal formulae with the
+;    same ScaledReg and Scale."
+;
+; Due to a bug in ARMTargetLowering::isLegalAddressingMode LSR got 
+; 4*reg({0,+,-1}) and -4*reg({0,+,-1}) had the same cost for the Thumb1 target.
+; Another issue was that LSR got that -1*reg was free for the Thumb1 target.
+
+; Test case 01: -1*reg is not free for the Thumb1 target.
+; 
+; CHECK-LABEL: @negativeOneCase
+; CHECK-NOT: mul
+; CHECK: ret i8
+define i8* @negativeOneCase(i8* returned %a, i8* nocapture readonly %b, i32 %n) nounwind {
+entry:
+  %add.ptr = getelementptr inbounds i8, i8* %a, i32 -1
+  br label %while.cond
+
+while.cond:                                       ; preds = %while.cond, %entry
+  %p.0 = phi i8* [ %add.ptr, %entry ], [ %incdec.ptr, %while.cond ]
+  %incdec.ptr = getelementptr inbounds i8, i8* %p.0, i32 1
+  %0 = load i8, i8* %incdec.ptr, align 1
+  %cmp = icmp eq i8 %0, 0
+  br i1 %cmp, label %while.cond2.preheader, label %while.cond
+
+while.cond2.preheader:                            ; preds = %while.cond
+  br label %while.cond2
+
+while.cond2:                                      ; preds = %while.cond2.preheader, %while.body5
+  %b.addr.0 = phi i8* [ %incdec.ptr6, %while.body5 ], [ %b, %while.cond2.preheader ]
+  %n.addr.0 = phi i32 [ %dec, %while.body5 ], [ %n, %while.cond2.preheader ]
+  %p.1 = phi i8* [ %incdec.ptr7, %while.body5 ], [ %incdec.ptr, %while.cond2.preheader ]
+  %cmp3 = icmp eq i32 %n.addr.0, 0
+  br i1 %cmp3, label %while.end8, label %while.body5
+
+while.body5:                                      ; preds = %while.cond2
+  %dec = add i32 %n.addr.0, -1
+  %incdec.ptr6 = getelementptr inbounds i8, i8* %b.addr.0, i32 1
+  %1 = load i8, i8* %b.addr.0, align 1
+  %incdec.ptr7 = getelementptr inbounds i8, i8* %p.1, i32 1
+  store i8 %1, i8* %p.1, align 1
+  br label %while.cond2
+
+while.end8:                                       ; preds = %while.cond2
+  %scevgep = getelementptr i8, i8* %incdec.ptr, i32 %n
+  store i8 0, i8* %scevgep, align 1
+  ret i8* %a
+}
+
+; Test case 02: 4*reg({0,+,-1}) and -4*reg({0,+,-1}) are not supported for
+;               the Thumb1 target.
+; 
+; CHECK-LABEL: @negativeFourCase
+; CHECK-NOT: mul
+; CHECK: ret void
+define void @negativeFourCase(i8* %ptr1, i32* %ptr2) nounwind {
+entry:
+  br label %for.cond6.preheader.us.i.i
+
+for.cond6.preheader.us.i.i:                       ; preds = %if.end48.us.i.i, %entry
+  %addr.0108.us.i.i = phi i8* [ %scevgep.i.i, %if.end48.us.i.i ], [ %ptr1, %entry ]
+  %inc49.us.i.i = phi i32 [ %inc50.us.i.i, %if.end48.us.i.i ], [ 0, %entry ]
+  %c1.0104.us.i.i = phi i32* [ %c0.0103.us.i.i, %if.end48.us.i.i ], [ %ptr2, %entry ]
+  %c0.0103.us.i.i = phi i32* [ %c1.0104.us.i.i, %if.end48.us.i.i ], [ %ptr2, %entry ]
+  br label %for.body8.us.i.i
+
+if.end48.us.i.i:                                  ; preds = %for.inc.us.i.i
+  %scevgep.i.i = getelementptr i8, i8* %addr.0108.us.i.i, i32 256
+  %inc50.us.i.i = add nuw nsw i32 %inc49.us.i.i, 1
+  %exitcond110.i.i = icmp eq i32 %inc50.us.i.i, 256
+  br i1 %exitcond110.i.i, label %exit.i, label %for.cond6.preheader.us.i.i
+
+for.body8.us.i.i:                                 ; preds = %for.inc.us.i.i, %for.cond6.preheader.us.i.i
+  %addr.198.us.i.i = phi i8* [ %addr.0108.us.i.i, %for.cond6.preheader.us.i.i ], [ %incdec.ptr.us.i.i, %for.inc.us.i.i ]
+  %inc.196.us.i.i = phi i32 [ 0, %for.cond6.preheader.us.i.i ], [ %inc.2.us.i.i, %for.inc.us.i.i ]
+  %c.093.us.i.i = phi i32 [ 0, %for.cond6.preheader.us.i.i ], [ %inc43.us.i.i, %for.inc.us.i.i ]
+  %incdec.ptr.us.i.i = getelementptr inbounds i8, i8* %addr.198.us.i.i, i32 1
+  %0 = load i8, i8* %addr.198.us.i.i, align 1
+  %cmp9.us.i.i = icmp eq i8 %0, -1
+  br i1 %cmp9.us.i.i, label %if.end37.us.i.i, label %if.else.us.i.i
+
+if.else.us.i.i:                                   ; preds = %for.body8.us.i.i
+  %add12.us.i.i = add nuw nsw i32 %c.093.us.i.i, 1
+  %arrayidx13.us.i.i = getelementptr inbounds i32, i32* %c1.0104.us.i.i, i32 %add12.us.i.i
+  %1 = load i32, i32* %arrayidx13.us.i.i, align 4
+  %arrayidx16.us.i.i = getelementptr inbounds i32, i32* %c1.0104.us.i.i, i32 %c.093.us.i.i
+  %2 = load i32, i32* %arrayidx16.us.i.i, align 4
+  %sub19.us.i.i = add nsw i32 %c.093.us.i.i, -1
+  %arrayidx20.us.i.i = getelementptr inbounds i32, i32* %c1.0104.us.i.i, i32 %sub19.us.i.i
+  %3 = load i32, i32* %arrayidx20.us.i.i, align 4
+  br label %if.end37.us.i.i
+
+if.end37.us.i.i:                                  ; preds = %if.else.us.i.i, %for.body8.us.i.i
+  %4 = phi i32 [ %3, %if.else.us.i.i ], [ 0, %for.body8.us.i.i ]
+  %arrayidx36.us.i.i = getelementptr inbounds i32, i32* %c0.0103.us.i.i, i32 %c.093.us.i.i
+  store i32 %4, i32* %arrayidx36.us.i.i, align 4
+  %inc.us.i.i = add nsw i32 %inc.196.us.i.i, 1
+  %cmp38.us.i.i = icmp sgt i32 %inc.196.us.i.i, 6
+  br i1 %cmp38.us.i.i, label %if.then40.us.i.i, label %for.inc.us.i.i
+
+if.then40.us.i.i:                                 ; preds = %if.end37.us.i.i
+  br label %for.inc.us.i.i
+
+for.inc.us.i.i:                                   ; preds = %if.then40.us.i.i, %if.end37.us.i.i
+  %inc.2.us.i.i = phi i32 [ 0, %if.then40.us.i.i ], [ %inc.us.i.i, %if.end37.us.i.i ]
+  %inc43.us.i.i = add nuw nsw i32 %c.093.us.i.i, 1
+  %exitcond.i.i = icmp eq i32 %inc43.us.i.i, 256
+  br i1 %exitcond.i.i, label %if.end48.us.i.i, label %for.body8.us.i.i
+
+exit.i:                               ; preds = %if.end48.us.i.i
+  ret void
+}
+




More information about the llvm-commits mailing list