From 93abd7d915fb12c9967fe2433dfc873d3b4e938d Mon Sep 17 00:00:00 2001 From: Leo Li Date: Mon, 10 Jul 2017 20:45:34 +0000 Subject: [PATCH] [ConstantHoisting] Remove dupliate logic in constant hoisting Summary: As metioned in https://reviews.llvm.org/D34576, checkings in `collectConstantCandidates` can be replaced by using `llvm::canReplaceOperandWithVariable`. The only special case is that `collectConstantCandidates` return false for all `IntrinsicInst` but it is safe for us to collect constant candidates from `IntrinsicInst`. Reviewers: pirama, efriedma, srhines Reviewed By: efriedma Subscribers: llvm-commits, javed.absar Differential Revision: https://reviews.llvm.org/D34921 llvm-svn: 307587 --- .../Transforms/Scalar/ConstantHoisting.cpp | 42 ++++--------------- llvm/lib/Transforms/Utils/Local.cpp | 3 ++ .../ConstantHoisting/ARM/bad-cases.ll | 31 ++++++++++++++ .../ConstantHoisting/ARM/insertvalue.ll | 31 ++++++++++++++ 4 files changed, 73 insertions(+), 34 deletions(-) create mode 100644 llvm/test/Transforms/ConstantHoisting/ARM/insertvalue.ll diff --git a/llvm/lib/Transforms/Scalar/ConstantHoisting.cpp b/llvm/lib/Transforms/Scalar/ConstantHoisting.cpp index 14638e85293d..122c9314e022 100644 --- a/llvm/lib/Transforms/Scalar/ConstantHoisting.cpp +++ b/llvm/lib/Transforms/Scalar/ConstantHoisting.cpp @@ -44,6 +44,7 @@ #include "llvm/Support/Debug.h" #include "llvm/Support/raw_ostream.h" #include "llvm/Transforms/Scalar.h" +#include "llvm/Transforms/Utils/Local.h" #include using namespace llvm; @@ -401,42 +402,15 @@ void ConstantHoistingPass::collectConstantCandidates( if (Inst->isCast()) return; - // Can't handle inline asm. Skip it. - if (auto Call = dyn_cast(Inst)) - if (isa(Call->getCalledValue())) - return; - - // Switch cases must remain constant, and if the value being tested is - // constant the entire thing should disappear. - if (isa(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(Inst); - if (AI && AI->isStaticAlloca()) - return; - - // Constants in GEPs that index into a struct type should not be hoisted. - if (isa(Inst)) { - gep_type_iterator GTI = gep_type_begin(Inst); - - // Collect constant for first operand. - collectConstantCandidates(ConstCandMap, Inst, 0); - // Scan rest operands. - for (unsigned Idx = 1, E = Inst->getNumOperands(); Idx != E; ++Idx, ++GTI) { - // Only collect constants that index into a non struct type. - if (!GTI.isStruct()) { - collectConstantCandidates(ConstCandMap, Inst, Idx); - } - } - return; - } - // Scan all operands. for (unsigned Idx = 0, E = Inst->getNumOperands(); Idx != E; ++Idx) { - collectConstantCandidates(ConstCandMap, Inst, Idx); + // The cost of materializing the constants (defined in + // `TargetTransformInfo::getIntImmCost`) for instructions which only take + // constant variables is lower than `TargetTransformInfo::TCC_Basic`. So + // it's safe for us to collect constant candidates from all IntrinsicInsts. + if (canReplaceOperandWithVariable(Inst, Idx) || isa(Inst)) { + collectConstantCandidates(ConstCandMap, Inst, Idx); + } } // end of for all operands } diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp index bc135852cd4f..74610613001c 100644 --- a/llvm/lib/Transforms/Utils/Local.cpp +++ b/llvm/lib/Transforms/Utils/Local.cpp @@ -2169,6 +2169,9 @@ bool llvm::canReplaceOperandWithVariable(const Instruction *I, unsigned OpIdx) { return true; case Instruction::Call: case Instruction::Invoke: + // Can't handle inline asm. Skip it. + if (isa(ImmutableCallSite(I).getCalledValue())) + return false; // Many arithmetic intrinsics have no issue taking a // variable, however it's hard to distingish these from // specials such as @llvm.frameaddress that require a constant. diff --git a/llvm/test/Transforms/ConstantHoisting/ARM/bad-cases.ll b/llvm/test/Transforms/ConstantHoisting/ARM/bad-cases.ll index ffcfb2e56c95..315e69998c62 100644 --- a/llvm/test/Transforms/ConstantHoisting/ARM/bad-cases.ll +++ b/llvm/test/Transforms/ConstantHoisting/ARM/bad-cases.ll @@ -107,3 +107,34 @@ entry: %ret = add i32 %cast0, %cast1 ret i32 %ret } + +@exception_type = external global i8 + +; Constants in inline ASM should not be hoisted. +define i32 @inline_asm_invoke() personality i8* null { +;CHECK-LABEL: @inline_asm_invoke +;CHECK-NOT: %const = 214672 +;CHECK: %X = invoke i32 asm "bswap $0", "=r,r"(i32 214672) + %X = invoke i32 asm "bswap $0", "=r,r"(i32 214672) + to label %L unwind label %lpad +;CHECK: %Y = invoke i32 asm "bswap $0", "=r,r"(i32 214672) + %Y = invoke i32 asm "bswap $0", "=r,r"(i32 214672) + to label %L unwind label %lpad +L: + ret i32 %X +lpad: + %lp = landingpad i32 + cleanup + catch i8* @exception_type + ret i32 1 +} + +define i32 @inline_asm_call() { +;CHECK-LABEL: @inline_asm_call +;CHECK-NOT: %const = 214672 +;CHECK: %X = call i32 asm "bswap $0", "=r,r"(i32 214672) + %X = call i32 asm "bswap $0", "=r,r"(i32 214672) +;CHECK: %Y = call i32 asm "bswap $0", "=r,r"(i32 214672) + %Y = call i32 asm "bswap $0", "=r,r"(i32 214672) + ret i32 %X +} diff --git a/llvm/test/Transforms/ConstantHoisting/ARM/insertvalue.ll b/llvm/test/Transforms/ConstantHoisting/ARM/insertvalue.ll new file mode 100644 index 000000000000..99fe7fbe22a5 --- /dev/null +++ b/llvm/test/Transforms/ConstantHoisting/ARM/insertvalue.ll @@ -0,0 +1,31 @@ +; RUN: opt -consthoist -S < %s | FileCheck %s +target triple = "thumbv6m-none-eabi" + +%T = type { i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, +i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, +i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, +i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, +i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, +i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, +i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, +i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, +i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, +i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, +i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, +i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, +i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, +i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, +i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, +i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, +i32, i32, i32, i32, i32, i32 } + +; The second operand of insertvalue is able to be hoisted. +define void @test1(%T %P) { +; CHECK-LABEL: @test1 +; CHECK: %const = bitcast i32 256 to i32 +; CHECK: %1 = insertvalue %T %P, i32 %const, 256 +; CHECK: %2 = insertvalue %T %P, i32 %const, 256 + %1 = insertvalue %T %P, i32 256, 256 + %2 = insertvalue %T %P, i32 256, 256 + ret void +}