-
Notifications
You must be signed in to change notification settings - Fork 15.1k
[HashRecognize] Forbid optz when data.next has exit-block user #165574
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
The CRC optimization relies on stripping the auxiliary data completely, and should hence be forbidden when it has a user in the exit-block. Forbid this case, fixing a miscompile. Fixes llvm#165382.
|
@llvm/pr-subscribers-llvm-analysis Author: Ramkumar Ramachandra (artagnon) ChangesThe CRC optimization relies on stripping the auxiliary data completely, and should hence be forbidden when it has a user in the exit-block. Forbid this case, fixing a miscompile. Fixes #165382. Full diff: https://github.com/llvm/llvm-project/pull/165574.diff 3 Files Affected:
diff --git a/llvm/lib/Analysis/HashRecognize.cpp b/llvm/lib/Analysis/HashRecognize.cpp
index 4529123508a7c..9fddb57effd4f 100644
--- a/llvm/lib/Analysis/HashRecognize.cpp
+++ b/llvm/lib/Analysis/HashRecognize.cpp
@@ -487,6 +487,14 @@ std::variant<PolynomialInfo, StringRef> HashRecognize::recognizeCRC() const {
: LHS->getType()->getIntegerBitWidth()))
return "Loop iterations exceed bitwidth of data";
+ // Make sure that the simple recurrence evolution isn't used in the exit
+ // block.
+ if (SimpleRecurrence && any_of(SimpleRecurrence.BO->users(), [Exit](User *U) {
+ auto *UI = dyn_cast<Instruction>(U);
+ return UI && UI->getParent() == Exit;
+ }))
+ return "Recurrences have stray uses";
+
// Make sure that the computed value is used in the exit block: this should be
// true even if it is only really used in an outer loop's exit block, since
// the loop is in LCSSA form.
diff --git a/llvm/test/Analysis/HashRecognize/cyclic-redundancy-check.ll b/llvm/test/Analysis/HashRecognize/cyclic-redundancy-check.ll
index 7dec2f8f96906..78b4139d21982 100644
--- a/llvm/test/Analysis/HashRecognize/cyclic-redundancy-check.ll
+++ b/llvm/test/Analysis/HashRecognize/cyclic-redundancy-check.ll
@@ -1448,4 +1448,85 @@ exit: ; preds = %loop
ret i16 %crc.next
}
+define i16 @not.crc.data.next.outside.user(i16 %crc.init, i16 %data.init) {
+; CHECK-LABEL: 'not.crc.data.next.outside.user'
+; CHECK-NEXT: Did not find a hash algorithm
+; CHECK-NEXT: Reason: Recurrences have stray uses
+;
+entry:
+ br label %loop
+
+loop:
+ %iv = phi i32 [ 0, %entry ], [ %iv.next, %loop ]
+ %crc = phi i16 [ %crc.init, %entry ], [ %crc.next, %loop ]
+ %data = phi i16 [ %data.init, %entry ], [ %data.next, %loop ]
+ %xor.crc.data = xor i16 %data, %crc
+ %crc.shl = shl i16 %crc, 1
+ %crc.xor = xor i16 %crc.shl, 3
+ %check.sb = icmp slt i16 %xor.crc.data, 0
+ %crc.next = select i1 %check.sb, i16 %crc.xor, i16 %crc.shl
+ %data.next = shl i16 %data, 1
+ %iv.next = add nuw nsw i32 %iv, 1
+ %exit.cond = icmp samesign ult i32 %iv, 7
+ br i1 %exit.cond, label %loop, label %exit
+
+exit:
+ %ret = xor i16 %data.next, %crc.next
+ ret i16 %ret
+}
+
+define i16 @not.crc.data.phi.outside.user(i16 %crc.init, i16 %data.init) {
+; CHECK-LABEL: 'not.crc.data.phi.outside.user'
+; CHECK-NEXT: Did not find a hash algorithm
+; CHECK-NEXT: Reason: Recurrences have stray uses
+;
+entry:
+ br label %loop
+
+loop:
+ %iv = phi i32 [ 0, %entry ], [ %iv.next, %loop ]
+ %crc = phi i16 [ %crc.init, %entry ], [ %crc.next, %loop ]
+ %data = phi i16 [ %data.init, %entry ], [ %data.next, %loop ]
+ %xor.crc.data = xor i16 %data, %crc
+ %crc.shl = shl i16 %crc, 1
+ %crc.xor = xor i16 %crc.shl, 3
+ %check.sb = icmp slt i16 %xor.crc.data, 0
+ %crc.next = select i1 %check.sb, i16 %crc.xor, i16 %crc.shl
+ %data.next = shl i16 %data, 1
+ %iv.next = add nuw nsw i32 %iv, 1
+ %exit.cond = icmp samesign ult i32 %iv, 7
+ br i1 %exit.cond, label %loop, label %exit
+
+exit:
+ %ret = xor i16 %data, %crc.next
+ ret i16 %ret
+}
+
+define i16 @not.crc.crc.phi.outside.user(i16 %crc.init, i16 %data.init) {
+; CHECK-LABEL: 'not.crc.crc.phi.outside.user'
+; CHECK-NEXT: Did not find a hash algorithm
+; CHECK-NEXT: Reason: Recurrences have stray uses
+;
+entry:
+ br label %loop
+
+loop:
+ %iv = phi i32 [ 0, %entry ], [ %iv.next, %loop ]
+ %crc = phi i16 [ %crc.init, %entry ], [ %crc.next, %loop ]
+ %data = phi i16 [ %data.init, %entry ], [ %data.next, %loop ]
+ %xor.crc.data = xor i16 %data, %crc
+ %crc.shl = shl i16 %crc, 1
+ %crc.xor = xor i16 %crc.shl, 3
+ %check.sb = icmp slt i16 %xor.crc.data, 0
+ %crc.next = select i1 %check.sb, i16 %crc.xor, i16 %crc.shl
+ %data.next = shl i16 %data, 1
+ %iv.next = add nuw nsw i32 %iv, 1
+ %exit.cond = icmp samesign ult i32 %iv, 7
+ br i1 %exit.cond, label %loop, label %exit
+
+exit:
+ %ret = xor i16 %crc, %crc.next
+ ret i16 %ret
+}
+
declare i16 @side.effect()
diff --git a/llvm/test/Transforms/LoopIdiom/cyclic-redundancy-check.ll b/llvm/test/Transforms/LoopIdiom/cyclic-redundancy-check.ll
index b2ec53ca405d4..90995a0257721 100644
--- a/llvm/test/Transforms/LoopIdiom/cyclic-redundancy-check.ll
+++ b/llvm/test/Transforms/LoopIdiom/cyclic-redundancy-check.ll
@@ -537,6 +537,52 @@ exit: ; preds = %loop
%ret = and i32 %unrelated.next, %crc.next
ret i32 %ret
}
+
+define i16 @not.crc.data.next.outside.user(i16 %crc.init, i16 %data.init) {
+; CHECK-LABEL: define i16 @not.crc.data.next.outside.user(
+; CHECK-SAME: i16 [[CRC_INIT:%.*]], i16 [[DATA_INIT:%.*]]) {
+; CHECK-NEXT: [[ENTRY:.*]]:
+; CHECK-NEXT: br label %[[LOOP:.*]]
+; CHECK: [[LOOP]]:
+; CHECK-NEXT: [[IV:%.*]] = phi i32 [ 0, %[[ENTRY]] ], [ [[IV_NEXT:%.*]], %[[LOOP]] ]
+; CHECK-NEXT: [[TBL_LD:%.*]] = phi i16 [ [[CRC_INIT]], %[[ENTRY]] ], [ [[CRC_NEXT:%.*]], %[[LOOP]] ]
+; CHECK-NEXT: [[CRC_BE_SHIFT:%.*]] = phi i16 [ [[DATA_INIT]], %[[ENTRY]] ], [ [[DATA_NEXT:%.*]], %[[LOOP]] ]
+; CHECK-NEXT: [[CRC_NEXT3:%.*]] = xor i16 [[CRC_BE_SHIFT]], [[TBL_LD]]
+; CHECK-NEXT: [[CRC_SHL:%.*]] = shl i16 [[TBL_LD]], 1
+; CHECK-NEXT: [[CRC_XOR:%.*]] = xor i16 [[CRC_SHL]], 3
+; CHECK-NEXT: [[CHECK_SB:%.*]] = icmp slt i16 [[CRC_NEXT3]], 0
+; CHECK-NEXT: [[CRC_NEXT]] = select i1 [[CHECK_SB]], i16 [[CRC_XOR]], i16 [[CRC_SHL]]
+; CHECK-NEXT: [[DATA_NEXT]] = shl i16 [[CRC_BE_SHIFT]], 1
+; CHECK-NEXT: [[IV_NEXT]] = add nuw nsw i32 [[IV]], 1
+; CHECK-NEXT: [[EXIT_COND:%.*]] = icmp samesign ult i32 [[IV]], 7
+; CHECK-NEXT: br i1 [[EXIT_COND]], label %[[LOOP]], label %[[EXIT:.*]]
+; CHECK: [[EXIT]]:
+; CHECK-NEXT: [[CRC_NEXT_LCSSA:%.*]] = phi i16 [ [[CRC_NEXT]], %[[LOOP]] ]
+; CHECK-NEXT: [[DATA_NEXT_LCSSA:%.*]] = phi i16 [ [[DATA_NEXT]], %[[LOOP]] ]
+; CHECK-NEXT: [[RET:%.*]] = xor i16 [[DATA_NEXT_LCSSA]], [[CRC_NEXT_LCSSA]]
+; CHECK-NEXT: ret i16 [[RET]]
+;
+entry:
+ br label %loop
+
+loop:
+ %iv = phi i32 [ 0, %entry ], [ %iv.next, %loop ]
+ %crc = phi i16 [ %crc.init, %entry ], [ %crc.next, %loop ]
+ %data = phi i16 [ %data.init, %entry ], [ %data.next, %loop ]
+ %xor.crc.data = xor i16 %data, %crc
+ %crc.shl = shl i16 %crc, 1
+ %crc.xor = xor i16 %crc.shl, 3
+ %check.sb = icmp slt i16 %xor.crc.data, 0
+ %crc.next = select i1 %check.sb, i16 %crc.xor, i16 %crc.shl
+ %data.next = shl i16 %data, 1
+ %iv.next = add nuw nsw i32 %iv, 1
+ %exit.cond = icmp samesign ult i32 %iv, 7
+ br i1 %exit.cond, label %loop, label %exit
+
+exit:
+ %ret = xor i16 %data.next, %crc.next
+ ret i16 %ret
+}
;.
; CHECK: attributes #[[ATTR0]] = { optsize }
;.
|
|
@llvm/pr-subscribers-llvm-transforms Author: Ramkumar Ramachandra (artagnon) ChangesThe CRC optimization relies on stripping the auxiliary data completely, and should hence be forbidden when it has a user in the exit-block. Forbid this case, fixing a miscompile. Fixes #165382. Full diff: https://github.com/llvm/llvm-project/pull/165574.diff 3 Files Affected:
diff --git a/llvm/lib/Analysis/HashRecognize.cpp b/llvm/lib/Analysis/HashRecognize.cpp
index 4529123508a7c..9fddb57effd4f 100644
--- a/llvm/lib/Analysis/HashRecognize.cpp
+++ b/llvm/lib/Analysis/HashRecognize.cpp
@@ -487,6 +487,14 @@ std::variant<PolynomialInfo, StringRef> HashRecognize::recognizeCRC() const {
: LHS->getType()->getIntegerBitWidth()))
return "Loop iterations exceed bitwidth of data";
+ // Make sure that the simple recurrence evolution isn't used in the exit
+ // block.
+ if (SimpleRecurrence && any_of(SimpleRecurrence.BO->users(), [Exit](User *U) {
+ auto *UI = dyn_cast<Instruction>(U);
+ return UI && UI->getParent() == Exit;
+ }))
+ return "Recurrences have stray uses";
+
// Make sure that the computed value is used in the exit block: this should be
// true even if it is only really used in an outer loop's exit block, since
// the loop is in LCSSA form.
diff --git a/llvm/test/Analysis/HashRecognize/cyclic-redundancy-check.ll b/llvm/test/Analysis/HashRecognize/cyclic-redundancy-check.ll
index 7dec2f8f96906..78b4139d21982 100644
--- a/llvm/test/Analysis/HashRecognize/cyclic-redundancy-check.ll
+++ b/llvm/test/Analysis/HashRecognize/cyclic-redundancy-check.ll
@@ -1448,4 +1448,85 @@ exit: ; preds = %loop
ret i16 %crc.next
}
+define i16 @not.crc.data.next.outside.user(i16 %crc.init, i16 %data.init) {
+; CHECK-LABEL: 'not.crc.data.next.outside.user'
+; CHECK-NEXT: Did not find a hash algorithm
+; CHECK-NEXT: Reason: Recurrences have stray uses
+;
+entry:
+ br label %loop
+
+loop:
+ %iv = phi i32 [ 0, %entry ], [ %iv.next, %loop ]
+ %crc = phi i16 [ %crc.init, %entry ], [ %crc.next, %loop ]
+ %data = phi i16 [ %data.init, %entry ], [ %data.next, %loop ]
+ %xor.crc.data = xor i16 %data, %crc
+ %crc.shl = shl i16 %crc, 1
+ %crc.xor = xor i16 %crc.shl, 3
+ %check.sb = icmp slt i16 %xor.crc.data, 0
+ %crc.next = select i1 %check.sb, i16 %crc.xor, i16 %crc.shl
+ %data.next = shl i16 %data, 1
+ %iv.next = add nuw nsw i32 %iv, 1
+ %exit.cond = icmp samesign ult i32 %iv, 7
+ br i1 %exit.cond, label %loop, label %exit
+
+exit:
+ %ret = xor i16 %data.next, %crc.next
+ ret i16 %ret
+}
+
+define i16 @not.crc.data.phi.outside.user(i16 %crc.init, i16 %data.init) {
+; CHECK-LABEL: 'not.crc.data.phi.outside.user'
+; CHECK-NEXT: Did not find a hash algorithm
+; CHECK-NEXT: Reason: Recurrences have stray uses
+;
+entry:
+ br label %loop
+
+loop:
+ %iv = phi i32 [ 0, %entry ], [ %iv.next, %loop ]
+ %crc = phi i16 [ %crc.init, %entry ], [ %crc.next, %loop ]
+ %data = phi i16 [ %data.init, %entry ], [ %data.next, %loop ]
+ %xor.crc.data = xor i16 %data, %crc
+ %crc.shl = shl i16 %crc, 1
+ %crc.xor = xor i16 %crc.shl, 3
+ %check.sb = icmp slt i16 %xor.crc.data, 0
+ %crc.next = select i1 %check.sb, i16 %crc.xor, i16 %crc.shl
+ %data.next = shl i16 %data, 1
+ %iv.next = add nuw nsw i32 %iv, 1
+ %exit.cond = icmp samesign ult i32 %iv, 7
+ br i1 %exit.cond, label %loop, label %exit
+
+exit:
+ %ret = xor i16 %data, %crc.next
+ ret i16 %ret
+}
+
+define i16 @not.crc.crc.phi.outside.user(i16 %crc.init, i16 %data.init) {
+; CHECK-LABEL: 'not.crc.crc.phi.outside.user'
+; CHECK-NEXT: Did not find a hash algorithm
+; CHECK-NEXT: Reason: Recurrences have stray uses
+;
+entry:
+ br label %loop
+
+loop:
+ %iv = phi i32 [ 0, %entry ], [ %iv.next, %loop ]
+ %crc = phi i16 [ %crc.init, %entry ], [ %crc.next, %loop ]
+ %data = phi i16 [ %data.init, %entry ], [ %data.next, %loop ]
+ %xor.crc.data = xor i16 %data, %crc
+ %crc.shl = shl i16 %crc, 1
+ %crc.xor = xor i16 %crc.shl, 3
+ %check.sb = icmp slt i16 %xor.crc.data, 0
+ %crc.next = select i1 %check.sb, i16 %crc.xor, i16 %crc.shl
+ %data.next = shl i16 %data, 1
+ %iv.next = add nuw nsw i32 %iv, 1
+ %exit.cond = icmp samesign ult i32 %iv, 7
+ br i1 %exit.cond, label %loop, label %exit
+
+exit:
+ %ret = xor i16 %crc, %crc.next
+ ret i16 %ret
+}
+
declare i16 @side.effect()
diff --git a/llvm/test/Transforms/LoopIdiom/cyclic-redundancy-check.ll b/llvm/test/Transforms/LoopIdiom/cyclic-redundancy-check.ll
index b2ec53ca405d4..90995a0257721 100644
--- a/llvm/test/Transforms/LoopIdiom/cyclic-redundancy-check.ll
+++ b/llvm/test/Transforms/LoopIdiom/cyclic-redundancy-check.ll
@@ -537,6 +537,52 @@ exit: ; preds = %loop
%ret = and i32 %unrelated.next, %crc.next
ret i32 %ret
}
+
+define i16 @not.crc.data.next.outside.user(i16 %crc.init, i16 %data.init) {
+; CHECK-LABEL: define i16 @not.crc.data.next.outside.user(
+; CHECK-SAME: i16 [[CRC_INIT:%.*]], i16 [[DATA_INIT:%.*]]) {
+; CHECK-NEXT: [[ENTRY:.*]]:
+; CHECK-NEXT: br label %[[LOOP:.*]]
+; CHECK: [[LOOP]]:
+; CHECK-NEXT: [[IV:%.*]] = phi i32 [ 0, %[[ENTRY]] ], [ [[IV_NEXT:%.*]], %[[LOOP]] ]
+; CHECK-NEXT: [[TBL_LD:%.*]] = phi i16 [ [[CRC_INIT]], %[[ENTRY]] ], [ [[CRC_NEXT:%.*]], %[[LOOP]] ]
+; CHECK-NEXT: [[CRC_BE_SHIFT:%.*]] = phi i16 [ [[DATA_INIT]], %[[ENTRY]] ], [ [[DATA_NEXT:%.*]], %[[LOOP]] ]
+; CHECK-NEXT: [[CRC_NEXT3:%.*]] = xor i16 [[CRC_BE_SHIFT]], [[TBL_LD]]
+; CHECK-NEXT: [[CRC_SHL:%.*]] = shl i16 [[TBL_LD]], 1
+; CHECK-NEXT: [[CRC_XOR:%.*]] = xor i16 [[CRC_SHL]], 3
+; CHECK-NEXT: [[CHECK_SB:%.*]] = icmp slt i16 [[CRC_NEXT3]], 0
+; CHECK-NEXT: [[CRC_NEXT]] = select i1 [[CHECK_SB]], i16 [[CRC_XOR]], i16 [[CRC_SHL]]
+; CHECK-NEXT: [[DATA_NEXT]] = shl i16 [[CRC_BE_SHIFT]], 1
+; CHECK-NEXT: [[IV_NEXT]] = add nuw nsw i32 [[IV]], 1
+; CHECK-NEXT: [[EXIT_COND:%.*]] = icmp samesign ult i32 [[IV]], 7
+; CHECK-NEXT: br i1 [[EXIT_COND]], label %[[LOOP]], label %[[EXIT:.*]]
+; CHECK: [[EXIT]]:
+; CHECK-NEXT: [[CRC_NEXT_LCSSA:%.*]] = phi i16 [ [[CRC_NEXT]], %[[LOOP]] ]
+; CHECK-NEXT: [[DATA_NEXT_LCSSA:%.*]] = phi i16 [ [[DATA_NEXT]], %[[LOOP]] ]
+; CHECK-NEXT: [[RET:%.*]] = xor i16 [[DATA_NEXT_LCSSA]], [[CRC_NEXT_LCSSA]]
+; CHECK-NEXT: ret i16 [[RET]]
+;
+entry:
+ br label %loop
+
+loop:
+ %iv = phi i32 [ 0, %entry ], [ %iv.next, %loop ]
+ %crc = phi i16 [ %crc.init, %entry ], [ %crc.next, %loop ]
+ %data = phi i16 [ %data.init, %entry ], [ %data.next, %loop ]
+ %xor.crc.data = xor i16 %data, %crc
+ %crc.shl = shl i16 %crc, 1
+ %crc.xor = xor i16 %crc.shl, 3
+ %check.sb = icmp slt i16 %xor.crc.data, 0
+ %crc.next = select i1 %check.sb, i16 %crc.xor, i16 %crc.shl
+ %data.next = shl i16 %data, 1
+ %iv.next = add nuw nsw i32 %iv, 1
+ %exit.cond = icmp samesign ult i32 %iv, 7
+ br i1 %exit.cond, label %loop, label %exit
+
+exit:
+ %ret = xor i16 %data.next, %crc.next
+ ret i16 %ret
+}
;.
; CHECK: attributes #[[ATTR0]] = { optsize }
;.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…165574) The CRC optimization relies on stripping the auxiliary data completely, and should hence be forbidden when it has a user in the exit-block. Forbid this case, fixing a miscompile. Fixes llvm#165382.
…165574) The CRC optimization relies on stripping the auxiliary data completely, and should hence be forbidden when it has a user in the exit-block. Forbid this case, fixing a miscompile. Fixes llvm#165382.
The CRC optimization relies on stripping the auxiliary data completely, and should hence be forbidden when it has a user in the exit-block. Forbid this case, fixing a miscompile.
Fixes #165382.