[llvm] r266260 - ARM: override cost function to re-enable ConstantHoisting (& fix it).
Arnaud De Grandmaison via llvm-commits
llvm-commits at lists.llvm.org
Fri Apr 15 06:27:49 PDT 2016
Hi Tim,
I have been bisecting some big performance regressions on SingleSource/Benchmarks/BenchmarkGame/Large/fasta and SingleSource/Benchmarks/Misc-C++/bigfib (see http://llvm.org/perf/db_default/v4/nts/daily_report/2016/4/14?filter-machine-regex=aarch64|arm|green&num_days=7), and it points to this commit.
Would you mind having a look at those ?
Kind regards,
Arnaud
> -----Original Message-----
> From: llvm-commits [mailto:llvm-commits-bounces at lists.llvm.org] On Behalf
> Of Tim Northover via llvm-commits
> Sent: 14 April 2016 00:08
> To: llvm-commits at lists.llvm.org
> Subject: [llvm] r266260 - ARM: override cost function to re-enable
> ConstantHoisting (& fix it).
>
> Author: tnorthover
> Date: Wed Apr 13 18:08:27 2016
> New Revision: 266260
>
> URL: http://llvm.org/viewvc/llvm-project?rev=266260&view=rev
> Log:
> ARM: override cost function to re-enable ConstantHoisting (& fix it).
>
> At some point, ARM stopped getting any benefit from ConstantHoisting
> because the pass called a different variant of getIntImmCost.
> Reimplementing the correct variant revealed some problems, however:
>
> + ConstantHoisting was modifying switch statements. This is simply invalid,
> the cases must remain integer constants no matter the notional cost.
> + ConstantHoisting was mangling alloca instructions in the entry block. These
> should be handled by FrameLowering, so constants actually have a cost of
> 0.
> Worse, the resulting bitcasts meant they became dynamic allocas.
>
> rdar://25707382
>
> Added:
> llvm/trunk/test/CodeGen/ARM/static-addr-hoisting.ll
> llvm/trunk/test/Transforms/ConstantHoisting/ARM/
> llvm/trunk/test/Transforms/ConstantHoisting/ARM/bad-cases.ll
> llvm/trunk/test/Transforms/ConstantHoisting/ARM/lit.local.cfg
> Modified:
> llvm/trunk/lib/Target/ARM/ARMTargetTransformInfo.cpp
> llvm/trunk/lib/Target/ARM/ARMTargetTransformInfo.h
> llvm/trunk/lib/Transforms/Scalar/ConstantHoisting.cpp
> llvm/trunk/test/CodeGen/ARM/lsr-code-insertion.ll
>
> Modified: llvm/trunk/lib/Target/ARM/ARMTargetTransformInfo.cpp
> URL: http://llvm.org/viewvc/llvm-
> project/llvm/trunk/lib/Target/ARM/ARMTargetTransformInfo.cpp?rev=2662
> 60&r1=266259&r2=266260&view=diff
> ==========================================================
> ====================
> --- llvm/trunk/lib/Target/ARM/ARMTargetTransformInfo.cpp (original)
> +++ llvm/trunk/lib/Target/ARM/ARMTargetTransformInfo.cpp Wed Apr 13
> +++ 18:08:27 2016
> @@ -18,12 +18,12 @@ using namespace llvm; int
> ARMTTIImpl::getIntImmCost(const APInt &Imm, Type *Ty) {
> assert(Ty->isIntegerTy());
>
> - unsigned Bits = Ty->getPrimitiveSizeInBits();
> - if (Bits == 0 || Bits > 32)
> - return 4;
> + unsigned Bits = Ty->getPrimitiveSizeInBits(); if (Bits == 0 || Bits >
> + 64)
> + return 4;
>
> - int32_t SImmVal = Imm.getSExtValue();
> - uint32_t ZImmVal = Imm.getZExtValue();
> + int64_t SImmVal = Imm.getSExtValue(); uint64_t ZImmVal =
> + Imm.getZExtValue();
> if (!ST->isThumb()) {
> if ((SImmVal >= 0 && SImmVal < 65536) ||
> (ARM_AM::getSOImmVal(ZImmVal) != -1) ||
>
> Modified: llvm/trunk/lib/Target/ARM/ARMTargetTransformInfo.h
> URL: http://llvm.org/viewvc/llvm-
> project/llvm/trunk/lib/Target/ARM/ARMTargetTransformInfo.h?rev=266260
> &r1=266259&r2=266260&view=diff
> ==========================================================
> ====================
> --- llvm/trunk/lib/Target/ARM/ARMTargetTransformInfo.h (original)
> +++ llvm/trunk/lib/Target/ARM/ARMTargetTransformInfo.h Wed Apr 13
> +++ 18:08:27 2016
> @@ -60,6 +60,10 @@ public:
> using BaseT::getIntImmCost;
> int getIntImmCost(const APInt &Imm, Type *Ty);
>
> + int getIntImmCost(unsigned Opcode, unsigned Idx, const APInt &Imm,
> Type *Ty) {
> + return getIntImmCost(Imm, Ty);
> + }
> +
> /// @}
>
> /// \name Vector TTI Implementations
>
> Modified: llvm/trunk/lib/Transforms/Scalar/ConstantHoisting.cpp
> URL: http://llvm.org/viewvc/llvm-
> project/llvm/trunk/lib/Transforms/Scalar/ConstantHoisting.cpp?rev=266260
> &r1=266259&r2=266260&view=diff
> ==========================================================
> ====================
> --- llvm/trunk/lib/Transforms/Scalar/ConstantHoisting.cpp (original)
> +++ llvm/trunk/lib/Transforms/Scalar/ConstantHoisting.cpp Wed Apr 13
> +++ 18:08:27 2016
> @@ -320,6 +320,18 @@ void ConstantHoisting::collectConstantCa
> if (isa<InlineAsm>(Call->getCalledValue()))
> return;
>
> + // Switch cases must remain constant, and if the value being tested
> + is // constant the entire thing should disappear.
> + if (isa<SwitchInst>(Inst))
> + return;
> +
> + // Static allocas (constant size in the entry block) are handled by
> + // prologue/epilogue insertion so they're free anyway. We definitely
> + don't // want to make them non-constant.
> + auto AI = dyn_cast<AllocaInst>(Inst); if (AI &&
> + AI->isStaticAlloca())
> + return;
> +
> // Scan all operands.
> for (unsigned Idx = 0, E = Inst->getNumOperands(); Idx != E; ++Idx) {
> Value *Opnd = Inst->getOperand(Idx);
>
> Modified: llvm/trunk/test/CodeGen/ARM/lsr-code-insertion.ll
> URL: http://llvm.org/viewvc/llvm-
> project/llvm/trunk/test/CodeGen/ARM/lsr-code-
> insertion.ll?rev=266260&r1=266259&r2=266260&view=diff
> ==========================================================
> ====================
> --- llvm/trunk/test/CodeGen/ARM/lsr-code-insertion.ll (original)
> +++ llvm/trunk/test/CodeGen/ARM/lsr-code-insertion.ll Wed Apr 13
> +++ 18:08:27 2016
> @@ -9,8 +9,8 @@
> ;
> ; CHECK: ldr [[R6:r[0-9*]+]], LCP
> ; CHECK: cmp {{.*}}, [[R6]]
> -; CHECK: ldrle
> -; CHECK-NEXT: strle
> +; CHECK-NOT: lt
> +; CHECK: strlt
>
> target triple = "arm-apple-darwin8"
>
>
> Added: llvm/trunk/test/CodeGen/ARM/static-addr-hoisting.ll
> URL: http://llvm.org/viewvc/llvm-
> project/llvm/trunk/test/CodeGen/ARM/static-addr-
> hoisting.ll?rev=266260&view=auto
> ==========================================================
> ====================
> --- llvm/trunk/test/CodeGen/ARM/static-addr-hoisting.ll (added)
> +++ llvm/trunk/test/CodeGen/ARM/static-addr-hoisting.ll Wed Apr 13
> +++ 18:08:27 2016
> @@ -0,0 +1,22 @@
> +; RUN: llc -mtriple=thumbv7-apple-ios %s -o - | FileCheck %s
> +
> +define void @multiple_store() {
> +; CHECK-LABEL: multiple_store:
> +; CHECK: movw r[[BASE1:[0-9]+]], #16960 ; CHECK: movs [[VAL:r[0-9]+]],
> +#42 ; CHECK: movt r[[BASE1]], #15
> +
> +; CHECK: str [[VAL]], [r[[BASE1]]]
> +; CHECK: str [[VAL]], [r[[BASE1]], #24] ; CHECK: str.w [[VAL]],
> +[r[[BASE1]], #42]
> +
> +; CHECK: movw r[[BASE2:[0-9]+]], #20394 ; CHECK: movt r[[BASE2]], #18
> +
> +; CHECK: str [[VAL]], [r[[BASE2]]]
> + store i32 42, i32* inttoptr(i32 1000000 to i32*)
> + store i32 42, i32* inttoptr(i32 1000024 to i32*)
> + store i32 42, i32* inttoptr(i32 1000042 to i32*)
> + store i32 42, i32* inttoptr(i32 1200042 to i32*)
> + ret void
> +}
>
> Added: llvm/trunk/test/Transforms/ConstantHoisting/ARM/bad-cases.ll
> URL: http://llvm.org/viewvc/llvm-
> project/llvm/trunk/test/Transforms/ConstantHoisting/ARM/bad-
> cases.ll?rev=266260&view=auto
> ==========================================================
> ====================
> --- llvm/trunk/test/Transforms/ConstantHoisting/ARM/bad-cases.ll (added)
> +++ llvm/trunk/test/Transforms/ConstantHoisting/ARM/bad-cases.ll Wed
> Apr
> +++ 13 18:08:27 2016
> @@ -0,0 +1,47 @@
> +; RUN: opt -consthoist -S < %s | FileCheck %s target triple =
> +"thumbv6m-none-eabi"
> +
> +; Allocas in the entry block get handled (for free) by ;
> +prologue/epilogue. Elsewhere they're fair game though.
> +define void @avoid_allocas() {
> +; CHECK-LABEL: @avoid_allocas
> +; CHECK: %addr1 = alloca i8, i32 1000
> +; CHECK: %addr2 = alloca i8, i32 1020
> +
> + %addr1 = alloca i8, i32 1000
> + %addr2 = alloca i8, i32 1020
> + br label %elsewhere
> +
> +elsewhere:
> +; CHECK: [[BASE:%.*]] = bitcast i32 1000 to i32 ; CHECK: alloca i8, i32
> +[[BASE]] ; CHECK: [[NEXT:%.*]] = add i32 [[BASE]], 20 ; CHECK: alloca
> +i8, i32 [[NEXT]]
> +
> + %addr3 = alloca i8, i32 1000
> + %addr4 = alloca i8, i32 1020
> +
> + ret void
> +}
> +
> +; The case values of switch instructions are required to be constants.
> +define void @avoid_switch(i32 %in) {
> +; CHECK-LABEL: @avoid_switch
> +; CHECK: switch i32 %in, label %default [
> +; CHECK: i32 1000, label %bb1
> +; CHECK: i32 1020, label %bb2
> +; CHECK: ]
> +
> + switch i32 %in, label %default
> + [ i32 1000, label %bb1
> + i32 1020, label %bb2 ]
> +
> +bb1:
> + ret void
> +
> +bb2:
> + ret void
> +
> +default:
> + ret void
> +}
>
> Added: llvm/trunk/test/Transforms/ConstantHoisting/ARM/lit.local.cfg
> URL: http://llvm.org/viewvc/llvm-
> project/llvm/trunk/test/Transforms/ConstantHoisting/ARM/lit.local.cfg?rev=
> 266260&view=auto
> ==========================================================
> ====================
> --- llvm/trunk/test/Transforms/ConstantHoisting/ARM/lit.local.cfg (added)
> +++ llvm/trunk/test/Transforms/ConstantHoisting/ARM/lit.local.cfg Wed
> +++ Apr 13 18:08:27 2016
> @@ -0,0 +1,2 @@
> +if not 'ARM' in config.root.targets:
> + config.unsupported = True
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list