[llvmbranchcommits] [llvmbranch] r168446  in /llvm/branches/release_32: ./ lib/Transforms/Scalar/Reassociate.cpp test/Transforms/Reassociate/crash.ll
Pawel Wodnicki
pawel at 32bitmicro.com
Wed Nov 21 10:55:44 PST 2012
Author: pawel
Date: Wed Nov 21 12:55:44 2012
New Revision: 168446
URL: http://llvm.org/viewvc/llvmproject?rev=168446&view=rev
Log:
Merging r168035: into 3.2 release branch.
Fix a crash observed by Shuxin Yang. The issue here is that LinearizeExprTree,
the utility for extracting a chain of operations from the IR, thought that it
might as well combine any constants it came across (rather than just returning
them along with everything else). On the other hand, the factorization code
would like to see the individual constants (this is quite reasonable: it is
much easier to pull a factor of 3 out of 2*3 than it is to pull it out of 6;
you may think 6/3 isn't so hard, but due to overflow it's not as easy to undo
multiplications of constants as it may at first appear). This patch therefore
makes LinearizeExprTree stupider: it now leaves optimizing to the optimization
part of reassociate, and sticks to just analysing the IR.
Modified:
llvm/branches/release_32/ (props changed)
llvm/branches/release_32/lib/Transforms/Scalar/Reassociate.cpp
llvm/branches/release_32/test/Transforms/Reassociate/crash.ll
Propchange: llvm/branches/release_32/

 svn:mergeinfo (original)
+++ svn:mergeinfo Wed Nov 21 12:55:44 2012
@@ 1,3 +1,3 @@
/llvm/branches/Apple/Pertwee:110850,110961
/llvm/branches/typesystemrewrite:133420134817
/llvm/trunk:155241,167718167719,167731,167737,167743,167750,167784,167811,167817,167855,167860167864,167875,167942,167948,167966,168001,168189,168197168198,168227,168316,168319,168346,168352,168361
+/llvm/trunk:155241,167718167719,167731,167737,167743,167750,167784,167811,167817,167855,167860167864,167875,167942,167948,167966,168001,168035,168189,168197168198,168227,168316,168319,168346,168352,168361
Modified: llvm/branches/release_32/lib/Transforms/Scalar/Reassociate.cpp
URL: http://llvm.org/viewvc/llvmproject/llvm/branches/release_32/lib/Transforms/Scalar/Reassociate.cpp?rev=168446&r1=168445&r2=168446&view=diff
==============================================================================
 llvm/branches/release_32/lib/Transforms/Scalar/Reassociate.cpp (original)
+++ llvm/branches/release_32/lib/Transforms/Scalar/Reassociate.cpp Wed Nov 21 12:55:44 2012
@@ 339,36 +339,6 @@
}
}
/// EvaluateRepeatedConstant  Compute C op C op ... op C where the constant C
/// is repeated Weight times.
static Constant *EvaluateRepeatedConstant(unsigned Opcode, Constant *C,
 APInt Weight) {
 // For addition the result can be efficiently computed as the product of the
 // constant and the weight.
 if (Opcode == Instruction::Add)
 return ConstantExpr::getMul(C, ConstantInt::get(C>getContext(), Weight));

 // The weight might be huge, so compute by repeated squaring to ensure that
 // compile time is proportional to the logarithm of the weight.
 Constant *Result = 0;
 Constant *Power = C; // Successively C, C op C, (C op C) op (C op C) etc.
 // Visit the bits in Weight.
 while (Weight != 0) {
 // If the current bit in Weight is nonzero do Result = Result op Power.
 if (Weight[0])
 Result = Result ? ConstantExpr::get(Opcode, Result, Power) : Power;
 // Move on to the next bit if any more are nonzero.
 Weight = Weight.lshr(1);
 if (Weight.isMinValue())
 break;
 // Square the power.
 Power = ConstantExpr::get(Opcode, Power, Power);
 }

 assert(Result && "Only positive weights supported!");
 return Result;
}

typedef std::pair<Value*, APInt> RepeatedValue;
/// LinearizeExprTree  Given an associative binary expression, return the leaf
@@ 382,9 +352,7 @@
/// op
/// (Ops[N].first op Ops[N].first op ... Ops[N].first) < Ops[N].second times
///
/// Note that the values Ops[0].first, ..., Ops[N].first are all distinct, and
/// they are all nonconstant except possibly for the last one, which if it is
/// constant will have weight one (Ops[N].second === 1).
+/// Note that the values Ops[0].first, ..., Ops[N].first are all distinct.
///
/// This routine may modify the function, in which case it returns 'true'. The
/// changes it makes may well be destructive, changing the value computed by 'I'
@@ 604,7 +572,6 @@
// The leaves, repeated according to their weights, represent the linearized
// form of the expression.
 Constant *Cst = 0; // Accumulate constants here.
for (unsigned i = 0, e = LeafOrder.size(); i != e; ++i) {
Value *V = LeafOrder[i];
LeafMap::iterator It = Leaves.find(V);
@@ 618,31 +585,14 @@
continue;
// Ensure the leaf is only output once.
It>second = 0;
 // Glob all constants together into Cst.
 if (Constant *C = dyn_cast<Constant>(V)) {
 C = EvaluateRepeatedConstant(Opcode, C, Weight);
 Cst = Cst ? ConstantExpr::get(Opcode, Cst, C) : C;
 continue;
 }
 // Add nonconstant
Ops.push_back(std::make_pair(V, Weight));
}
 // Add any constants back into Ops, all globbed together and reduced to having
 // weight 1 for the convenience of users.
 Constant *Identity = ConstantExpr::getBinOpIdentity(Opcode, I>getType());
 if (Cst && Cst != Identity) {
 // If combining multiple constants resulted in the absorber then the entire
 // expression must evaluate to the absorber.
 if (Cst == Absorber)
 Ops.clear();
 Ops.push_back(std::make_pair(Cst, APInt(Bitwidth, 1)));
 }

// For nilpotent operations or addition there may be no operands, for example
// because the expression was "X xor X" or consisted of 2^Bitwidth additions:
// in both cases the weight reduces to 0 causing the value to be skipped.
if (Ops.empty()) {
+ Constant *Identity = ConstantExpr::getBinOpIdentity(Opcode, I>getType());
assert(Identity && "Associative operation without identity!");
Ops.push_back(std::make_pair(Identity, APInt(Bitwidth, 1)));
}
@@ 1446,9 +1396,26 @@
SmallVectorImpl<ValueEntry> &Ops) {
// Now that we have the linearized expression tree, try to optimize it.
// Start by folding any constants that we found.
 if (Ops.size() == 1) return Ops[0].Op;

+ Constant *Cst = 0;
unsigned Opcode = I>getOpcode();
+ while (!Ops.empty() && isa<Constant>(Ops.back().Op)) {
+ Constant *C = cast<Constant>(Ops.pop_back_val().Op);
+ Cst = Cst ? ConstantExpr::get(Opcode, C, Cst) : C;
+ }
+ // If there was nothing but constants then we are done.
+ if (Ops.empty())
+ return Cst;
+
+ // Put the combined constant back at the end of the operand list, except if
+ // there is no point. For example, an add of 0 gets dropped here, while a
+ // multiplication by zero turns the whole expression into zero.
+ if (Cst && Cst != ConstantExpr::getBinOpIdentity(Opcode, I>getType())) {
+ if (Cst == ConstantExpr::getBinOpAbsorber(Opcode, I>getType()))
+ return Cst;
+ Ops.push_back(ValueEntry(0, Cst));
+ }
+
+ if (Ops.size() == 1) return Ops[0].Op;
// Handle destructive annihilation due to identities between elements in the
// argument list here.
Modified: llvm/branches/release_32/test/Transforms/Reassociate/crash.ll
URL: http://llvm.org/viewvc/llvmproject/llvm/branches/release_32/test/Transforms/Reassociate/crash.ll?rev=168446&r1=168445&r2=168446&view=diff
==============================================================================
 llvm/branches/release_32/test/Transforms/Reassociate/crash.ll (original)
+++ llvm/branches/release_32/test/Transforms/Reassociate/crash.ll Wed Nov 21 12:55:44 2012
@@ 144,3 +144,12 @@
%t6 = add i32 %t4, %t5
ret i32 %t6
}
+
+define i32 @bar(i32 %arg, i32 %arg1, i32 %arg2) {
+ %tmp1 = mul i32 %arg1, 2
+ %tmp2 = mul i32 %tmp1, 3
+ %tmp3 = mul i32 %arg2, 2
+ %tmp4 = add i32 %tmp1, 1 ; dead code
+ %ret = add i32 %tmp2, %tmp3
+ ret i32 %ret
+}
More information about the llvmbranchcommits
mailing list