<div dir="ltr">Hi Hiroshi,<div><br></div><div>unfortunately, this is creating an assertion failure in a (somewhat old version of) SpiderMonkey. I have already forwarded reproduction instructions to you. For now, I have reverted this patch in r309659.</div><div><br></div><div>Cheers,</div><div>Daniel</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Jul 28, 2017 at 8:35 PM, Hiroshi Yamauchi via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: yamauchi<br>
Date: Fri Jul 28 11:35:25 2017<br>
New Revision: 309415<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=309415&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project?rev=309415&view=rev</a><br>
Log:<br>
[LVI] Constant-propagate a zero extension of the switch condition value through case edges<br>
<br>
Summary:<br>
LazyValueInfo currently computes the constant value of the switch condition through case edges, which allows the constant value to be propagated through the case edges.<br>
<br>
But we have seen a case where a zero-extended value of the switch condition is used past case edges for which the constant propagation doesn't occur.<br>
<br>
This patch adds a small logic to handle such a case in getEdgeValueLocal().<br>
<br>
This is motivated by the Python 2.7 eval loop in PyEval_EvalFrameEx() where the lack of the constant propagation causes longer live ranges and more spill code than necessary.<br>
<br>
With this patch, we see that the code size of PyEval_EvalFrameEx() decreases by ~5.4% and a performance test improves by ~4.6%.<br>
<br>
<br>
<br>
<br>
Reviewers: wmi, dberlin, sanjoy<br>
<br>
Reviewed By: sanjoy<br>
<br>
Subscribers: davide, davidxl, llvm-commits<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D34822" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D34822</a><br>
<br>
Modified:<br>
llvm/trunk/lib/Analysis/<wbr>LazyValueInfo.cpp<br>
llvm/trunk/test/Transforms/<wbr>CorrelatedValuePropagation/<wbr>range.ll<br>
<br>
Modified: llvm/trunk/lib/Analysis/<wbr>LazyValueInfo.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/LazyValueInfo.cpp?rev=309415&r1=309414&r2=309415&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/lib/<wbr>Analysis/LazyValueInfo.cpp?<wbr>rev=309415&r1=309414&r2=<wbr>309415&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/lib/Analysis/<wbr>LazyValueInfo.cpp (original)<br>
+++ llvm/trunk/lib/Analysis/<wbr>LazyValueInfo.cpp Fri Jul 28 11:35:25 2017<br>
@@ -17,6 +17,7 @@<br>
#include "llvm/ADT/STLExtras.h"<br>
#include "llvm/Analysis/<wbr>AssumptionCache.h"<br>
#include "llvm/Analysis/<wbr>ConstantFolding.h"<br>
+#include "llvm/Analysis/<wbr>InstructionSimplify.h"<br>
#include "llvm/Analysis/<wbr>TargetLibraryInfo.h"<br>
#include "llvm/Analysis/ValueTracking.<wbr>h"<br>
#include "llvm/IR/<wbr>AssemblyAnnotationWriter.h"<br>
@@ -148,6 +149,15 @@ public:<br>
return Range;<br>
}<br>
<br>
+ Optional<APInt> asConstantInteger() const {<br>
+ if (isConstant() && isa<ConstantInt>(Val)) {<br>
+ return Val->getUniqueInteger();<br>
+ } else if (isConstantRange() && Range.isSingleElement()) {<br>
+ return *Range.getSingleElement();<br>
+ }<br>
+ return Optional<APInt>();<br>
+ }<br>
+<br>
private:<br>
void markOverdefined() {<br>
if (isOverdefined())<br>
@@ -1355,6 +1365,42 @@ LVILatticeVal getValueFromCondition(Valu<br>
return getValueFromCondition(Val, Cond, isTrueDest, Visited);<br>
}<br>
<br>
+// Return true if Usr has Op as an operand, otherwise false.<br>
+static bool usesOperand(User *Usr, Value *Op) {<br>
+ return find(Usr->operands(), Op) != Usr->op_end();<br>
+}<br>
+<br>
+// Check if Val can be simplified to an integer constant when the value of one<br>
+// of its operands Op is an integer constant OpConstVal. If so, return it as an<br>
+// lattice value range with a single element or otherwise return an overdefined<br>
+// lattice value.<br>
+static LVILatticeVal constantFoldUser(Value *Val, Value *Op,<br>
+ const APInt &OpConstVal,<br>
+ const DataLayout &DL) {<br>
+ Constant* OpConst = Constant::getIntegerValue(Op-><wbr>getType(), OpConstVal);<br>
+ // Check if Val can be simplified to a constant.<br>
+ if (auto *CI = dyn_cast<CastInst>(Val)) {<br>
+ assert(CI->getOperand(0) == Op && "Operand 0 isn't Op");<br>
+ if (auto *C = dyn_cast_or_null<ConstantInt>(<br>
+ SimplifyCastInst(CI-><wbr>getOpcode(), OpConst,<br>
+ CI->getDestTy(), DL))) {<br>
+ return LVILatticeVal::getRange(<wbr>ConstantRange(C-><wbr>getUniqueInteger()));<br>
+ }<br>
+ } else if (auto *BO = dyn_cast<BinaryOperator>(Val)) {<br>
+ bool Op0Match = BO->getOperand(0) == Op;<br>
+ bool Op1Match = BO->getOperand(1) == Op;<br>
+ assert((Op0Match || Op1Match) &&<br>
+ "Operand 0 nor Operand 1 isn't a match");<br>
+ Value *LHS = Op0Match ? OpConst : BO->getOperand(0);<br>
+ Value *RHS = Op1Match ? OpConst : BO->getOperand(1);<br>
+ if (auto *C = dyn_cast_or_null<ConstantInt>(<br>
+ SimplifyBinOp(BO->getOpcode(), LHS, RHS, DL))) {<br>
+ return LVILatticeVal::getRange(<wbr>ConstantRange(C-><wbr>getUniqueInteger()));<br>
+ }<br>
+ }<br>
+ return LVILatticeVal::getOverdefined(<wbr>);<br>
+}<br>
+<br>
/// \brief Compute the value of Val on the edge BBFrom -> BBTo. Returns false if<br>
/// Val is not constrained on the edge. Result is unspecified if return value<br>
/// is false.<br>
@@ -1370,10 +1416,11 @@ static bool getEdgeValueLocal(Value *Val<br>
bool isTrueDest = BI->getSuccessor(0) == BBTo;<br>
assert(BI->getSuccessor(!<wbr>isTrueDest) == BBTo &&<br>
"BBTo isn't a successor of BBFrom");<br>
+ Value *Condition = BI->getCondition();<br>
<br>
// If V is the condition of the branch itself, then we know exactly what<br>
// it is.<br>
- if (BI->getCondition() == Val) {<br>
+ if (Condition == Val) {<br>
Result = LVILatticeVal::get(<wbr>ConstantInt::get(<br>
Type::getInt1Ty(Val-><wbr>getContext()), isTrueDest));<br>
return true;<br>
@@ -1381,7 +1428,44 @@ static bool getEdgeValueLocal(Value *Val<br>
<br>
// If the condition of the branch is an equality comparison, we may be<br>
// able to infer the value.<br>
- Result = getValueFromCondition(Val, BI->getCondition(), isTrueDest);<br>
+ Result = getValueFromCondition(Val, Condition, isTrueDest);<br>
+ if (!Result.isOverdefined())<br>
+ return true;<br>
+<br>
+ if (User *Usr = dyn_cast<User>(Val)) {<br>
+ assert(Result.isOverdefined() && "Result isn't overdefined");<br>
+ if (isa<IntegerType>(Val-><wbr>getType())) {<br>
+ const DataLayout &DL = BBTo->getModule()-><wbr>getDataLayout();<br>
+ if (usesOperand(Usr, Condition)) {<br>
+ // If Val has Condition as an operand and Val can be folded into a<br>
+ // constant with either Condition == true or Condition == false,<br>
+ // propagate the constant.<br>
+ // eg.<br>
+ // ; %Val is true on the edge to %then.<br>
+ // %Val = and i1 %Condition, true.<br>
+ // br %Condition, label %then, label %else<br>
+ APInt ConditionVal(1, isTrueDest ? 1 : 0);<br>
+ Result = constantFoldUser(Val, Condition, ConditionVal, DL);<br>
+ } else {<br>
+ // If one of Val's operand has an inferred value, we may be able to<br>
+ // infer the value of Val.<br>
+ // eg.<br>
+ // ; %Val is 94 on the edge to %then.<br>
+ // %Val = add i8 %Op, 1<br>
+ // %Condition = icmp eq i8 %Op, 93<br>
+ // br i1 %Condition, label %then, label %else<br>
+ for (unsigned i = 0; i < Usr->getNumOperands(); ++i) {<br>
+ Value *Op = Usr->getOperand(i);<br>
+ LVILatticeVal OpLatticeVal =<br>
+ getValueFromCondition(Op, Condition, isTrueDest);<br>
+ if (Optional<APInt> OpConst = OpLatticeVal.<wbr>asConstantInteger()) {<br>
+ Result = constantFoldUser(Val, Op, OpConst.getValue(), DL);<br>
+ break;<br>
+ }<br>
+ }<br>
+ }<br>
+ }<br>
+ }<br>
if (!Result.isOverdefined())<br>
return true;<br>
}<br>
@@ -1390,15 +1474,35 @@ static bool getEdgeValueLocal(Value *Val<br>
// If the edge was formed by a switch on the value, then we may know exactly<br>
// what it is.<br>
if (SwitchInst *SI = dyn_cast<SwitchInst>(BBFrom-><wbr>getTerminator())) {<br>
- if (SI->getCondition() != Val)<br>
+ Value *Condition = SI->getCondition();<br>
+ if (!isa<IntegerType>(Val-><wbr>getType()))<br>
return false;<br>
+ bool ValUsesCondition = false;<br>
+ if (Condition != Val) {<br>
+ // Check if Val has Condition as an operand.<br>
+ if (User *Usr = dyn_cast<User>(Val))<br>
+ ValUsesCondition = usesOperand(Usr, Condition);<br>
+ if (!ValUsesCondition)<br>
+ return false;<br>
+ }<br>
+ assert((Condition == Val || ValUsesCondition) &&<br>
+ "Condition != Val nor Val doesn't use Condition");<br>
<br>
bool DefaultCase = SI->getDefaultDest() == BBTo;<br>
unsigned BitWidth = Val->getType()-><wbr>getIntegerBitWidth();<br>
ConstantRange EdgesVals(BitWidth, DefaultCase/*isFullSet*/);<br>
<br>
for (auto Case : SI->cases()) {<br>
- ConstantRange EdgeVal(Case.getCaseValue()-><wbr>getValue());<br>
+ APInt CaseValue = Case.getCaseValue()->getValue(<wbr>);<br>
+ ConstantRange EdgeVal(CaseValue);<br>
+ if (ValUsesCondition) {<br>
+ const DataLayout &DL = BBTo->getModule()-><wbr>getDataLayout();<br>
+ LVILatticeVal EdgeLatticeVal =<br>
+ constantFoldUser(Val, Condition, CaseValue, DL);<br>
+ if (EdgeLatticeVal.isOverdefined(<wbr>))<br>
+ return false;<br>
+ EdgeVal = EdgeLatticeVal.<wbr>getConstantRange();<br>
+ }<br>
if (DefaultCase) {<br>
// It is possible that the default destination is the destination of<br>
// some cases. There is no need to perform difference for those cases.<br>
<br>
Modified: llvm/trunk/test/Transforms/<wbr>CorrelatedValuePropagation/<wbr>range.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/CorrelatedValuePropagation/range.ll?rev=309415&r1=309414&r2=309415&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/test/<wbr>Transforms/<wbr>CorrelatedValuePropagation/<wbr>range.ll?rev=309415&r1=309414&<wbr>r2=309415&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/test/Transforms/<wbr>CorrelatedValuePropagation/<wbr>range.ll (original)<br>
+++ llvm/trunk/test/Transforms/<wbr>CorrelatedValuePropagation/<wbr>range.ll Fri Jul 28 11:35:25 2017<br>
@@ -462,3 +462,117 @@ then:<br>
else:<br>
ret i1 false<br>
}<br>
+<br>
+define i32 @test16(i8 %a) {<br>
+entry:<br>
+ %b = zext i8 %a to i32<br>
+ br label %dispatch<br>
+<br>
+dispatch:<br>
+ %cmp = icmp eq i8 %a, 93<br>
+ br i1 %cmp, label %target93, label %dispatch<br>
+<br>
+; CHECK-LABEL: @test16(<br>
+; CHECK: target93:<br>
+; CHECK-NEXT: ret i32 93<br>
+target93:<br>
+ ret i32 %b<br>
+}<br>
+<br>
+define i32 @test16_i1(i1 %a) {<br>
+entry:<br>
+ %b = zext i1 %a to i32<br>
+ br label %dispatch<br>
+<br>
+dispatch:<br>
+ br i1 %a, label %true, label %dispatch<br>
+<br>
+; CHECK-LABEL: @test16_i1(<br>
+; CHECK: true:<br>
+; CHECK-NEXT: ret i32 1<br>
+true:<br>
+ ret i32 %b<br>
+}<br>
+<br>
+define i8 @test17(i8 %a) {<br>
+entry:<br>
+ %c = add i8 %a, 3<br>
+ br label %dispatch<br>
+<br>
+dispatch:<br>
+ %cmp = icmp eq i8 %a, 93<br>
+ br i1 %cmp, label %target93, label %dispatch<br>
+<br>
+; CHECK-LABEL: @test17(<br>
+; CHECK: target93:<br>
+; CHECK-NEXT: ret i8 96<br>
+target93:<br>
+ ret i8 %c<br>
+}<br>
+<br>
+define i8 @test17_2(i8 %a) {<br>
+entry:<br>
+ %c = add i8 %a, %a<br>
+ br label %dispatch<br>
+<br>
+dispatch:<br>
+ %cmp = icmp eq i8 %a, 93<br>
+ br i1 %cmp, label %target93, label %dispatch<br>
+<br>
+; CHECK-LABEL: @test17_2(<br>
+; CHECK: target93:<br>
+; CHECK-NEXT: ret i8 -70<br>
+target93:<br>
+ ret i8 %c<br>
+}<br>
+<br>
+define i1 @test17_i1(i1 %a) {<br>
+entry:<br>
+ %c = and i1 %a, true<br>
+ br label %dispatch<br>
+<br>
+dispatch:<br>
+ br i1 %a, label %true, label %dispatch<br>
+<br>
+; CHECK-LABEL: @test17_i1(<br>
+; CHECK: true:<br>
+; CHECK-NEXT: ret i1 true<br>
+true:<br>
+ ret i1 %c<br>
+}<br>
+<br>
+define i32 @test18(i8 %a) {<br>
+entry:<br>
+ %b = zext i8 %a to i32<br>
+ br label %dispatch<br>
+<br>
+dispatch:<br>
+ switch i8 %a, label %dispatch [<br>
+ i8 93, label %target93<br>
+ i8 -111, label %dispatch<br>
+ ]<br>
+<br>
+; CHECK-LABEL: @test18(<br>
+; CHECK: target93:<br>
+; CHECK-NEXT: ret i32 93<br>
+target93:<br>
+ ret i32 %b<br>
+}<br>
+<br>
+define i8 @test19(i8 %a) {<br>
+entry:<br>
+ %c = add i8 %a, 3<br>
+ br label %dispatch<br>
+<br>
+dispatch:<br>
+ switch i8 %a, label %dispatch [<br>
+ i8 93, label %target93<br>
+ i8 -111, label %dispatch<br>
+ ]<br>
+<br>
+; CHECK-LABEL: @test19(<br>
+; CHECK: target93:<br>
+; CHECK-NEXT: ret i8 96<br>
+target93:<br>
+ ret i8 %c<br>
+}<br>
<br>
<br>
______________________________<wbr>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div>