[PATCH] D32718: [NewGVN] Don't derive incorrect implications.
Davide Italiano via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon May 1 14:11:08 PDT 2017
davide created this revision.
Herald added a subscriber: Prazek.
The problem is here (debug output):
Processing instruction %tmp4 = icmp sgt i32 %tmp.0, 0
Created new congruence class for %tmp4 = icmp sgt i32 %tmp.0, 0 using expression { ExpressionTypeConstant, opcode = 18, constant = i1 true} at 4 and leader i1 true
New class 4 for expression { ExpressionTypeConstant, opcode = 18, constant = i1 true}
we believe %tmp1 implies %tmp4
where:
br i1 %tmp1, label %bb2, label %bb7
br i1 %tmp4, label %bb5, label %bb7
because while looking at PredicateInfo stuffs we end up calling isImpliedTrueByMatchingCmp() with the operands swapped,
`y less then x` implies `y less then or equal x` but the other way around is not true.
The function has a very confusing name IMHO. I expect `isImpliedTrueByMatchingCmp` means that *the first argument* is implied by the second and not viceversa. I'm willing to change it, but I wonder if we have other latent bugs in tree because of this.
https://reviews.llvm.org/D32718
Files:
lib/Transforms/Scalar/NewGVN.cpp
test/Transforms/NewGVN/pr32852.ll
Index: test/Transforms/NewGVN/pr32852.ll
===================================================================
--- /dev/null
+++ test/Transforms/NewGVN/pr32852.ll
@@ -0,0 +1,24 @@
+; Make sure GVN doesn't incorrectly think the branch terminating
+; bb2 has a constant condition.
+; RUN: opt -S -newgvn %s | FileCheck %s
+
+ at a = common global i32 0
+ at .str = private unnamed_addr constant [3 x i8] c"0\0A\00"
+
+define void @main() {
+bb:
+ %tmp = load i32, i32* @a
+ %tmp1 = icmp sge i32 %tmp, 0
+ br i1 %tmp1, label %bb2, label %bb7
+bb2:
+ %tmp4 = icmp sgt i32 %tmp, 0
+; CHECK: br i1 %tmp4, label %bb5, label %bb7
+ br i1 %tmp4, label %bb5, label %bb7
+bb5:
+ %tmp6 = call i32 (i8*, ...) @printf(i8* getelementptr inbounds ([3 x i8], [3 x i8]* @.str, i32 0, i32 0))
+ br label %bb7
+bb7:
+ ret void
+}
+
+declare i32 @printf(i8*, ...)
Index: lib/Transforms/Scalar/NewGVN.cpp
===================================================================
--- lib/Transforms/Scalar/NewGVN.cpp
+++ lib/Transforms/Scalar/NewGVN.cpp
@@ -1628,15 +1628,15 @@
if (PBranch->TrueEdge) {
// If we know the previous predicate is true and we are in the true
// edge then we may be implied true or false.
- if (CmpInst::isImpliedTrueByMatchingCmp(OurPredicate,
- BranchPredicate)) {
+ if (CmpInst::isImpliedTrueByMatchingCmp(BranchPredicate,
+ OurPredicate)) {
addPredicateUsers(PI, I);
return createConstantExpression(
ConstantInt::getTrue(CI->getType()));
}
- if (CmpInst::isImpliedFalseByMatchingCmp(OurPredicate,
- BranchPredicate)) {
+ if (CmpInst::isImpliedFalseByMatchingCmp(BranchPredicate,
+ OurPredicate)) {
addPredicateUsers(PI, I);
return createConstantExpression(
ConstantInt::getFalse(CI->getType()));
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D32718.97339.patch
Type: text/x-patch
Size: 2063 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170501/96eb0a7e/attachment.bin>
More information about the llvm-commits
mailing list