[IR] Forbid mixing condition and operand bundle assumes #160460
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Assumes either have a boolean condition, or a number of attribute based operand bundles. Currently, we also allow mixing both forms, though we don't make use of this in practice. This adds additional complexity for code dealing with assumes. Forbid mixing both forms, by requiring that assumes with operand bundles have an i1 true condition.
llvmbot
added
llvm:instcombine
|
@llvm/pr-subscribers-mlir-llvm @llvm/pr-subscribers-llvm-transforms Author: Nikita Popov (nikic) ChangesAssumes either have a boolean condition, or a number of attribute based operand bundles. Currently, we also allow mixing both forms, though we don't make use of this in practice. This adds additional complexity for code dealing with assumes. Forbid mixing both forms, by requiring that assumes with operand bundles have an i1 true condition. Full diff: https://github.com/llvm/llvm-project/pull/160460.diff 5 Files Affected:
diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst
index b32a27f9555fd..41c2c1a9a0416 100644
--- a/llvm/docs/LangRef.rst
+++ b/llvm/docs/LangRef.rst
@@ -3013,6 +3013,8 @@ assumptions, such as that a :ref:`parameter attribute <paramattrs>` or a
location. Operand bundles enable assumptions that are either hard or impossible
to represent as a boolean argument of an :ref:`llvm.assume <int_assume>`.
+Assumes with operand bundles must have ``i1 true`` as the condition operand.
+
An assume operand bundle has the form:
::
@@ -3045,7 +3047,7 @@ allows the optimizer to assume that at location of call to
.. code-block:: llvm
- call void @llvm.assume(i1 %cond) ["cold"(), "nonnull"(ptr %val)]
+ call void @llvm.assume(i1 true) ["cold"(), "nonnull"(ptr %val)]
allows the optimizer to assume that the :ref:`llvm.assume <int_assume>`
call location is cold and that ``%val`` may not be null.
diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index 9bde965d660a4..0c6175b1945cc 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -5675,6 +5675,11 @@ void Verifier::visitIntrinsicCall(Intrinsic::ID ID, CallBase &Call) {
default:
break;
case Intrinsic::assume: {
+ if (Call.hasOperandBundles()) {
+ auto *Cond = dyn_cast<ConstantInt>(Call.getArgOperand(0));
+ Check(Cond && Cond->isOne(),
+ "assume with operand bundles must have i1 true condition", Call);
+ }
for (auto &Elem : Call.bundle_op_infos()) {
unsigned ArgCount = Elem.End - Elem.Begin;
// Separate storage assumptions are special insofar as they're the only
diff --git a/llvm/test/Transforms/AlignmentFromAssumptions/domtree-crash.ll b/llvm/test/Transforms/AlignmentFromAssumptions/domtree-crash.ll
index c7fc1dc699671..f9b9dd13b0d0c 100644
--- a/llvm/test/Transforms/AlignmentFromAssumptions/domtree-crash.ll
+++ b/llvm/test/Transforms/AlignmentFromAssumptions/domtree-crash.ll
@@ -9,10 +9,10 @@
define void @fn1() {
; CHECK-LABEL: define void @fn1() {
-; CHECK-NEXT: call void @llvm.assume(i1 false) [ "align"(ptr @global, i64 1) ]
+; CHECK-NEXT: call void @llvm.assume(i1 true) [ "align"(ptr @global, i64 1) ]
; CHECK-NEXT: ret void
;
- call void @llvm.assume(i1 false) [ "align"(ptr @global, i64 1) ]
+ call void @llvm.assume(i1 true) [ "align"(ptr @global, i64 1) ]
ret void
}
diff --git a/llvm/test/Transforms/InstCombine/assume.ll b/llvm/test/Transforms/InstCombine/assume.ll
index e87a61a57ea47..c6c1e597654c5 100644
--- a/llvm/test/Transforms/InstCombine/assume.ll
+++ b/llvm/test/Transforms/InstCombine/assume.ll
@@ -495,30 +495,6 @@ not_taken:
ret i1 %rval.2
}
-define i1 @nonnull3B(ptr %a, i1 %control) {
-; CHECK-LABEL: @nonnull3B(
-; CHECK-NEXT: entry:
-; CHECK-NEXT: br i1 [[CONTROL:%.*]], label [[TAKEN:%.*]], label [[NOT_TAKEN:%.*]]
-; CHECK: taken:
-; CHECK-NEXT: [[LOAD:%.*]] = load ptr, ptr [[A:%.*]], align 8
-; CHECK-NEXT: [[CMP:%.*]] = icmp ne ptr [[LOAD]], null
-; CHECK-NEXT: call void @llvm.assume(i1 [[CMP]]) [ "nonnull"(ptr [[LOAD]]) ]
-; CHECK-NEXT: ret i1 [[CMP]]
-; CHECK: not_taken:
-; CHECK-NEXT: ret i1 false
-;
-entry:
- %load = load ptr, ptr %a
- %cmp = icmp ne ptr %load, null
- br i1 %control, label %taken, label %not_taken
-taken:
- call void @llvm.assume(i1 %cmp) ["nonnull"(ptr %load)]
- ret i1 %cmp
-not_taken:
- call void @llvm.assume(i1 %cmp) ["nonnull"(ptr %load)]
- ret i1 %control
-}
-
declare i1 @tmp1(i1)
define i1 @nonnull3C(ptr %a, i1 %control) {
@@ -544,7 +520,7 @@ taken:
br label %exit
exit:
; FIXME: this shouldn't be dropped because it is still dominated by the new position of %load
- call void @llvm.assume(i1 %cmp) ["nonnull"(ptr %load)]
+ call void @llvm.assume(i1 %cmp)
ret i1 %cmp2
not_taken:
call void @llvm.assume(i1 %cmp)
@@ -575,7 +551,7 @@ taken:
exit:
ret i1 %cmp2
not_taken:
- call void @llvm.assume(i1 %cmp) ["nonnull"(ptr %load)]
+ call void @llvm.assume(i1 %cmp)
ret i1 %control
}
diff --git a/llvm/test/Verifier/assume-bundles.ll b/llvm/test/Verifier/assume-bundles.ll
index d8037b965edb5..728b118c99fb6 100644
--- a/llvm/test/Verifier/assume-bundles.ll
+++ b/llvm/test/Verifier/assume-bundles.ll
@@ -3,7 +3,7 @@
declare void @llvm.assume(i1)
-define void @func(ptr %P, i32 %P1, ptr %P2, ptr %P3) {
+define void @func(ptr %P, i32 %P1, ptr %P2, ptr %P3, i1 %cond) {
; CHECK: tags must be valid attribute names
; CHECK: "adazdazd"
call void @llvm.assume(i1 true) ["adazdazd"()]
@@ -32,5 +32,7 @@ define void @func(ptr %P, i32 %P1, ptr %P2, ptr %P3) {
call void @llvm.assume(i1 true) ["separate_storage"(ptr %P, i32 123)]
; CHECK: dereferenceable assumptions should have 2 arguments
call void @llvm.assume(i1 true) ["align"(ptr %P, i32 4), "dereferenceable"(ptr %P)]
+; CHECK: assume with operand bundles must have i1 true condition
+ call void @llvm.assume(i1 %cond) ["nonnull"(ptr %P)]
ret void
}
|
|
@llvm/pr-subscribers-llvm-ir Author: Nikita Popov (nikic) ChangesAssumes either have a boolean condition, or a number of attribute based operand bundles. Currently, we also allow mixing both forms, though we don't make use of this in practice. This adds additional complexity for code dealing with assumes. Forbid mixing both forms, by requiring that assumes with operand bundles have an i1 true condition. Full diff: https://github.com/llvm/llvm-project/pull/160460.diff 5 Files Affected:
diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst
index b32a27f9555fd..41c2c1a9a0416 100644
--- a/llvm/docs/LangRef.rst
+++ b/llvm/docs/LangRef.rst
@@ -3013,6 +3013,8 @@ assumptions, such as that a :ref:`parameter attribute <paramattrs>` or a
location. Operand bundles enable assumptions that are either hard or impossible
to represent as a boolean argument of an :ref:`llvm.assume <int_assume>`.
+Assumes with operand bundles must have ``i1 true`` as the condition operand.
+
An assume operand bundle has the form:
::
@@ -3045,7 +3047,7 @@ allows the optimizer to assume that at location of call to
.. code-block:: llvm
- call void @llvm.assume(i1 %cond) ["cold"(), "nonnull"(ptr %val)]
+ call void @llvm.assume(i1 true) ["cold"(), "nonnull"(ptr %val)]
allows the optimizer to assume that the :ref:`llvm.assume <int_assume>`
call location is cold and that ``%val`` may not be null.
diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index 9bde965d660a4..0c6175b1945cc 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -5675,6 +5675,11 @@ void Verifier::visitIntrinsicCall(Intrinsic::ID ID, CallBase &Call) {
default:
break;
case Intrinsic::assume: {
+ if (Call.hasOperandBundles()) {
+ auto *Cond = dyn_cast<ConstantInt>(Call.getArgOperand(0));
+ Check(Cond && Cond->isOne(),
+ "assume with operand bundles must have i1 true condition", Call);
+ }
for (auto &Elem : Call.bundle_op_infos()) {
unsigned ArgCount = Elem.End - Elem.Begin;
// Separate storage assumptions are special insofar as they're the only
diff --git a/llvm/test/Transforms/AlignmentFromAssumptions/domtree-crash.ll b/llvm/test/Transforms/AlignmentFromAssumptions/domtree-crash.ll
index c7fc1dc699671..f9b9dd13b0d0c 100644
--- a/llvm/test/Transforms/AlignmentFromAssumptions/domtree-crash.ll
+++ b/llvm/test/Transforms/AlignmentFromAssumptions/domtree-crash.ll
@@ -9,10 +9,10 @@
define void @fn1() {
; CHECK-LABEL: define void @fn1() {
-; CHECK-NEXT: call void @llvm.assume(i1 false) [ "align"(ptr @global, i64 1) ]
+; CHECK-NEXT: call void @llvm.assume(i1 true) [ "align"(ptr @global, i64 1) ]
; CHECK-NEXT: ret void
;
- call void @llvm.assume(i1 false) [ "align"(ptr @global, i64 1) ]
+ call void @llvm.assume(i1 true) [ "align"(ptr @global, i64 1) ]
ret void
}
diff --git a/llvm/test/Transforms/InstCombine/assume.ll b/llvm/test/Transforms/InstCombine/assume.ll
index e87a61a57ea47..c6c1e597654c5 100644
--- a/llvm/test/Transforms/InstCombine/assume.ll
+++ b/llvm/test/Transforms/InstCombine/assume.ll
@@ -495,30 +495,6 @@ not_taken:
ret i1 %rval.2
}
-define i1 @nonnull3B(ptr %a, i1 %control) {
-; CHECK-LABEL: @nonnull3B(
-; CHECK-NEXT: entry:
-; CHECK-NEXT: br i1 [[CONTROL:%.*]], label [[TAKEN:%.*]], label [[NOT_TAKEN:%.*]]
-; CHECK: taken:
-; CHECK-NEXT: [[LOAD:%.*]] = load ptr, ptr [[A:%.*]], align 8
-; CHECK-NEXT: [[CMP:%.*]] = icmp ne ptr [[LOAD]], null
-; CHECK-NEXT: call void @llvm.assume(i1 [[CMP]]) [ "nonnull"(ptr [[LOAD]]) ]
-; CHECK-NEXT: ret i1 [[CMP]]
-; CHECK: not_taken:
-; CHECK-NEXT: ret i1 false
-;
-entry:
- %load = load ptr, ptr %a
- %cmp = icmp ne ptr %load, null
- br i1 %control, label %taken, label %not_taken
-taken:
- call void @llvm.assume(i1 %cmp) ["nonnull"(ptr %load)]
- ret i1 %cmp
-not_taken:
- call void @llvm.assume(i1 %cmp) ["nonnull"(ptr %load)]
- ret i1 %control
-}
-
declare i1 @tmp1(i1)
define i1 @nonnull3C(ptr %a, i1 %control) {
@@ -544,7 +520,7 @@ taken:
br label %exit
exit:
; FIXME: this shouldn't be dropped because it is still dominated by the new position of %load
- call void @llvm.assume(i1 %cmp) ["nonnull"(ptr %load)]
+ call void @llvm.assume(i1 %cmp)
ret i1 %cmp2
not_taken:
call void @llvm.assume(i1 %cmp)
@@ -575,7 +551,7 @@ taken:
exit:
ret i1 %cmp2
not_taken:
- call void @llvm.assume(i1 %cmp) ["nonnull"(ptr %load)]
+ call void @llvm.assume(i1 %cmp)
ret i1 %control
}
diff --git a/llvm/test/Verifier/assume-bundles.ll b/llvm/test/Verifier/assume-bundles.ll
index d8037b965edb5..728b118c99fb6 100644
--- a/llvm/test/Verifier/assume-bundles.ll
+++ b/llvm/test/Verifier/assume-bundles.ll
@@ -3,7 +3,7 @@
declare void @llvm.assume(i1)
-define void @func(ptr %P, i32 %P1, ptr %P2, ptr %P3) {
+define void @func(ptr %P, i32 %P1, ptr %P2, ptr %P3, i1 %cond) {
; CHECK: tags must be valid attribute names
; CHECK: "adazdazd"
call void @llvm.assume(i1 true) ["adazdazd"()]
@@ -32,5 +32,7 @@ define void @func(ptr %P, i32 %P1, ptr %P2, ptr %P3) {
call void @llvm.assume(i1 true) ["separate_storage"(ptr %P, i32 123)]
; CHECK: dereferenceable assumptions should have 2 arguments
call void @llvm.assume(i1 true) ["align"(ptr %P, i32 4), "dereferenceable"(ptr %P)]
+; CHECK: assume with operand bundles must have i1 true condition
+ call void @llvm.assume(i1 %cond) ["nonnull"(ptr %P)]
ret void
}
|
artagnon
reviewed
Sep 24, 2025
Comment on lines
515
to
519
| call void @llvm.assume(i1 %cmp) ["nonnull"(ptr %load)] | ||
| ret i1 %cmp | ||
| not_taken: | ||
| call void @llvm.assume(i1 %cmp) ["nonnull"(ptr %load)] | ||
| ret i1 %control |
There was a problem hiding this comment.
Might be good to keep this test, but split the assume into two assumes?
There was a problem hiding this comment.
I've restored this test with just the bundles, without the conditions (as other tests cover those).
fhahn
approved these changes
Sep 24, 2025
| case Intrinsic::assume: { | ||
| if (Call.hasOperandBundles()) { | ||
| auto *Cond = dyn_cast<ConstantInt>(Call.getArgOperand(0)); | ||
| Check(Cond && Cond->isOne(), |
There was a problem hiding this comment.
I guess this could use m_One(), but not sure if it would be really more compact
There was a problem hiding this comment.
I considered this, but we currently don't use PatternMatch at all inside the Verifier, so I stuck with the existing style.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Assumes either have a boolean condition, or a number of attribute based operand bundles. Currently, we also allow mixing both forms, though we don't make use of this in practice. This adds additional complexity for code dealing with assumes.
Forbid mixing both forms, by requiring that assumes with operand bundles have an i1 true condition.