From 2079d678b5bdb7114c9caad69168696b24bf13ff Mon Sep 17 00:00:00 2001 From: Leo Li Date: Thu, 29 Jun 2017 17:03:34 +0000 Subject: [PATCH] [ConstantHoisting] Avoid hoisting constants in GEPs that index into a struct type. Summary: Indices for GEPs that index into a struct type should always be constants. This added more checks in `collectConstantCandidates:` which make sure constants for GEP pointer type are not hoisted. This fixed Bug https://bugs.llvm.org/show_bug.cgi?id=33538 Reviewers: ributzka, rnk Reviewed By: ributzka Subscribers: efriedma, llvm-commits, srhines, javed.absar, pirama Differential Revision: https://reviews.llvm.org/D34576 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@306704 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/Transforms/Scalar/ConstantHoisting.h | 2 + lib/Transforms/Scalar/ConstantHoisting.cpp | 95 ++++++++++++++-------- .../ConstantHoisting/ARM/gep-struct-index.ll | 37 +++++++++ 3 files changed, 99 insertions(+), 35 deletions(-) create mode 100644 test/Transforms/ConstantHoisting/ARM/gep-struct-index.ll diff --git a/include/llvm/Transforms/Scalar/ConstantHoisting.h b/include/llvm/Transforms/Scalar/ConstantHoisting.h index edc91add7a7..a2a9afc083a 100644 --- a/include/llvm/Transforms/Scalar/ConstantHoisting.h +++ b/include/llvm/Transforms/Scalar/ConstantHoisting.h @@ -132,6 +132,8 @@ private: Instruction *Inst, unsigned Idx, ConstantInt *ConstInt); void collectConstantCandidates(ConstCandMapType &ConstCandMap, + Instruction *Inst, unsigned Idx); + void collectConstantCandidates(ConstCandMapType &ConstCandMap, Instruction *Inst); void collectConstantCandidates(Function &Fn); void findAndMakeBaseConstant(ConstCandVecType::iterator S, diff --git a/lib/Transforms/Scalar/ConstantHoisting.cpp b/lib/Transforms/Scalar/ConstantHoisting.cpp index c3810366bf2..a49c9b68c97 100644 --- a/lib/Transforms/Scalar/ConstantHoisting.cpp +++ b/lib/Transforms/Scalar/ConstantHoisting.cpp @@ -38,6 +38,7 @@ #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/Statistic.h" #include "llvm/IR/Constants.h" +#include "llvm/IR/GetElementPtrTypeIterator.h" #include "llvm/IR/IntrinsicInst.h" #include "llvm/Pass.h" #include "llvm/Support/Debug.h" @@ -340,6 +341,49 @@ void ConstantHoistingPass::collectConstantCandidates( } } + +/// \brief Check the operand for instruction Inst at index Idx. +void ConstantHoistingPass::collectConstantCandidates( + ConstCandMapType &ConstCandMap, Instruction *Inst, unsigned Idx) { + Value *Opnd = Inst->getOperand(Idx); + + // Visit constant integers. + if (auto ConstInt = dyn_cast(Opnd)) { + collectConstantCandidates(ConstCandMap, Inst, Idx, ConstInt); + return; + } + + // Visit cast instructions that have constant integers. + if (auto CastInst = dyn_cast(Opnd)) { + // Only visit cast instructions, which have been skipped. All other + // instructions should have already been visited. + if (!CastInst->isCast()) + return; + + if (auto *ConstInt = dyn_cast(CastInst->getOperand(0))) { + // Pretend the constant is directly used by the instruction and ignore + // the cast instruction. + collectConstantCandidates(ConstCandMap, Inst, Idx, ConstInt); + return; + } + } + + // Visit constant expressions that have constant integers. + if (auto ConstExpr = dyn_cast(Opnd)) { + // Only visit constant cast expressions. + if (!ConstExpr->isCast()) + return; + + if (auto ConstInt = dyn_cast(ConstExpr->getOperand(0))) { + // Pretend the constant is directly used by the instruction and ignore + // the constant expression. + collectConstantCandidates(ConstCandMap, Inst, Idx, ConstInt); + return; + } + } +} + + /// \brief Scan the instruction for expensive integer constants and record them /// in the constant candidate vector. void ConstantHoistingPass::collectConstantCandidates( @@ -365,44 +409,25 @@ void ConstantHoistingPass::collectConstantCandidates( if (AI && AI->isStaticAlloca()) return; - // Scan all operands. - for (unsigned Idx = 0, E = Inst->getNumOperands(); Idx != E; ++Idx) { - Value *Opnd = Inst->getOperand(Idx); - - // Visit constant integers. - if (auto ConstInt = dyn_cast(Opnd)) { - collectConstantCandidates(ConstCandMap, Inst, Idx, ConstInt); - continue; - } - - // Visit cast instructions that have constant integers. - if (auto CastInst = dyn_cast(Opnd)) { - // Only visit cast instructions, which have been skipped. All other - // instructions should have already been visited. - if (!CastInst->isCast()) - continue; - - if (auto *ConstInt = dyn_cast(CastInst->getOperand(0))) { - // Pretend the constant is directly used by the instruction and ignore - // the cast instruction. - collectConstantCandidates(ConstCandMap, Inst, Idx, ConstInt); - continue; + // 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; + } - // Visit constant expressions that have constant integers. - if (auto ConstExpr = dyn_cast(Opnd)) { - // Only visit constant cast expressions. - if (!ConstExpr->isCast()) - continue; - - if (auto ConstInt = dyn_cast(ConstExpr->getOperand(0))) { - // Pretend the constant is directly used by the instruction and ignore - // the constant expression. - collectConstantCandidates(ConstCandMap, Inst, Idx, ConstInt); - continue; - } - } + // Scan all operands. + for (unsigned Idx = 0, E = Inst->getNumOperands(); Idx != E; ++Idx) { + collectConstantCandidates(ConstCandMap, Inst, Idx); } // end of for all operands } diff --git a/test/Transforms/ConstantHoisting/ARM/gep-struct-index.ll b/test/Transforms/ConstantHoisting/ARM/gep-struct-index.ll new file mode 100644 index 00000000000..45f4500b37c --- /dev/null +++ b/test/Transforms/ConstantHoisting/ARM/gep-struct-index.ll @@ -0,0 +1,37 @@ +; 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 } + +; Indices for GEPs that index into a struct type should not be hoisted. +define i32 @test1(%T* %P) nounwind { +; CHECK-LABEL: @test1 +; CHECK: %const = bitcast i32 256 to i32 +; CHECK: %addr1 = getelementptr %T, %T* %P, i32 %const, i32 256 +; CHECK: %addr2 = getelementptr %T, %T* %P, i32 %const, i32 256 +; The first index into the pointer is hoisted, but the second one into the +; struct isn't. + %addr1 = getelementptr %T, %T* %P, i32 256, i32 256 + %tmp1 = load i32, i32* %addr1 + %addr2 = getelementptr %T, %T* %P, i32 256, i32 256 + %tmp2 = load i32, i32* %addr2 + %tmp4 = add i32 %tmp1, %tmp2 + ret i32 %tmp4 +} + -- 2.11.0