[llvm] r306704 - [ConstantHoisting] Avoid hoisting constants in GEPs that index into a struct type.
Leo Li via llvm-commits
llvm-commits at lists.llvm.org
Thu Jun 29 10:03:34 PDT 2017
Author: aoli
Date: Thu Jun 29 10:03:34 2017
New Revision: 306704
URL: http://llvm.org/viewvc/llvm-project?rev=306704&view=rev
Log:
[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
Added:
llvm/trunk/test/Transforms/ConstantHoisting/ARM/gep-struct-index.ll
Modified:
llvm/trunk/include/llvm/Transforms/Scalar/ConstantHoisting.h
llvm/trunk/lib/Transforms/Scalar/ConstantHoisting.cpp
Modified: llvm/trunk/include/llvm/Transforms/Scalar/ConstantHoisting.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Transforms/Scalar/ConstantHoisting.h?rev=306704&r1=306703&r2=306704&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Transforms/Scalar/ConstantHoisting.h (original)
+++ llvm/trunk/include/llvm/Transforms/Scalar/ConstantHoisting.h Thu Jun 29 10:03:34 2017
@@ -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,
Modified: llvm/trunk/lib/Transforms/Scalar/ConstantHoisting.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/ConstantHoisting.cpp?rev=306704&r1=306703&r2=306704&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/ConstantHoisting.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/ConstantHoisting.cpp Thu Jun 29 10:03:34 2017
@@ -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::collectConsta
}
}
+
+/// \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<ConstantInt>(Opnd)) {
+ collectConstantCandidates(ConstCandMap, Inst, Idx, ConstInt);
+ return;
+ }
+
+ // Visit cast instructions that have constant integers.
+ if (auto CastInst = dyn_cast<Instruction>(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<ConstantInt>(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<ConstantExpr>(Opnd)) {
+ // Only visit constant cast expressions.
+ if (!ConstExpr->isCast())
+ return;
+
+ if (auto ConstInt = dyn_cast<ConstantInt>(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::collectConsta
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<ConstantInt>(Opnd)) {
- collectConstantCandidates(ConstCandMap, Inst, Idx, ConstInt);
- continue;
- }
-
- // Visit cast instructions that have constant integers.
- if (auto CastInst = dyn_cast<Instruction>(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<ConstantInt>(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<GetElementPtrInst>(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<ConstantExpr>(Opnd)) {
- // Only visit constant cast expressions.
- if (!ConstExpr->isCast())
- continue;
-
- if (auto ConstInt = dyn_cast<ConstantInt>(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
}
Added: llvm/trunk/test/Transforms/ConstantHoisting/ARM/gep-struct-index.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/ConstantHoisting/ARM/gep-struct-index.ll?rev=306704&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/ConstantHoisting/ARM/gep-struct-index.ll (added)
+++ llvm/trunk/test/Transforms/ConstantHoisting/ARM/gep-struct-index.ll Thu Jun 29 10:03:34 2017
@@ -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
+}
+
More information about the llvm-commits
mailing list