From bf02c23ee9c1ec4a6dda35c3941f0cfba6919221 Mon Sep 17 00:00:00 2001 From: Evgeny Astigeevich Date: Thu, 24 Aug 2017 10:00:25 +0000 Subject: [PATCH] [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 llvm-svn: 311649 --- lib/Target/ARM/ARMISelLowering.cpp | 22 +++- lib/Target/ARM/ARMISelLowering.h | 4 + test/CodeGen/Thumb/addr-modes.ll | 45 +++++++ .../LoopStrengthReduce/illegal-addr-modes.ll | 122 ++++++++++++++++++ 4 files changed, 189 insertions(+), 4 deletions(-) create mode 100644 test/CodeGen/Thumb/addr-modes.ll create mode 100644 test/Transforms/LoopStrengthReduce/illegal-addr-modes.ll diff --git a/lib/Target/ARM/ARMISelLowering.cpp b/lib/Target/ARM/ARMISelLowering.cpp index 03d0b7e2a74..8d04f59b95c 100644 --- a/lib/Target/ARM/ARMISelLowering.cpp +++ b/lib/Target/ARM/ARMISelLowering.cpp @@ -12383,6 +12383,21 @@ bool ARMTargetLowering::isLegalT2ScaledAddressingMode(const AddrMode &AM, } } +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::isLegalAddressingMode(const DataLayout &DL, 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::isLegalAddressingMode(const DataLayout &DL, if (!VT.isSimple()) return false; + if (Subtarget->isThumb1Only()) + return isLegalT1ScaledAddressingMode(AM, VT); + if (Subtarget->isThumb2()) return isLegalT2ScaledAddressingMode(AM, VT); diff --git a/lib/Target/ARM/ARMISelLowering.h b/lib/Target/ARM/ARMISelLowering.h index 7761aa1315e..fd93629f084 100644 --- a/lib/Target/ARM/ARMISelLowering.h +++ b/lib/Target/ARM/ARMISelLowering.h @@ -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 diff --git a/test/CodeGen/Thumb/addr-modes.ll b/test/CodeGen/Thumb/addr-modes.ll new file mode 100644 index 00000000000..e6ed01d0547 --- /dev/null +++ b/test/CodeGen/Thumb/addr-modes.ll @@ -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 +} + diff --git a/test/Transforms/LoopStrengthReduce/illegal-addr-modes.ll b/test/Transforms/LoopStrengthReduce/illegal-addr-modes.ll new file mode 100644 index 00000000000..cb17d587961 --- /dev/null +++ b/test/Transforms/LoopStrengthReduce/illegal-addr-modes.ll @@ -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 +} +