-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[MergeFunc] Handle ConstantRange attributes. #88584
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
@llvm/pr-subscribers-llvm-transforms Author: Andreas Jonson (andjo403) ChangesIt is possible to update AttributeImpl::operator< instead but feels strange to say that a range is less then an other range. CC @nikic Full diff: https://github.com/llvm/llvm-project/pull/88584.diff 2 Files Affected:
diff --git a/llvm/lib/Transforms/Utils/FunctionComparator.cpp b/llvm/lib/Transforms/Utils/FunctionComparator.cpp
index 8cfb8ed07d240e..0c520ec7c5d2dd 100644
--- a/llvm/lib/Transforms/Utils/FunctionComparator.cpp
+++ b/llvm/lib/Transforms/Utils/FunctionComparator.cpp
@@ -143,6 +143,14 @@ int FunctionComparator::cmpAttrs(const AttributeList L,
if (int Res = cmpNumbers((uint64_t)TyL, (uint64_t)TyR))
return Res;
continue;
+ } else if (LA.isConstantRangeAttribute() &&
+ RA.isConstantRangeAttribute()) {
+ ConstantRange LCR = LA.getRange();
+ ConstantRange RCR = RA.getRange();
+ if (int Res = cmpAPInts(LCR.getLower(), RCR.getLower()))
+ return Res;
+ if (int Res = cmpAPInts(LCR.getUpper(), RCR.getUpper()))
+ return Res;
}
if (LA < RA)
return -1;
diff --git a/llvm/test/Transforms/MergeFunc/call-and-invoke-with-ranges-attr.ll b/llvm/test/Transforms/MergeFunc/call-and-invoke-with-ranges-attr.ll
new file mode 100644
index 00000000000000..e5d62319bf9db7
--- /dev/null
+++ b/llvm/test/Transforms/MergeFunc/call-and-invoke-with-ranges-attr.ll
@@ -0,0 +1,111 @@
+; RUN: opt -passes=mergefunc -S < %s | FileCheck %s
+
+define i8 @call_with_range_attr(i8 range(i8 0, 2) %v) {
+ %out = call i8 @dummy2(i8 %v)
+ ret i8 %out
+}
+
+define i8 @call_no_range_attr(i8 %v) {
+; CHECK-LABEL: @call_no_range_attr
+; CHECK-NEXT: %out = call i8 @dummy2(i8 %v)
+; CHECK-NEXT: ret i8 %out
+ %out = call i8 @dummy2(i8 %v)
+ ret i8 %out
+}
+
+define i8 @call_different_range_attr(i8 range(i8 5, 7) %v) {
+; CHECK-LABEL: @call_different_range_attr
+; CHECK-NEXT: %out = call i8 @dummy2(i8 %v)
+; CHECK-NEXT: ret i8 %out
+ %out = call i8 @dummy2(i8 %v)
+ ret i8 %out
+}
+
+define i8 @call_with_range() {
+ %out = call range(i8 0, 2) i8 @dummy()
+ ret i8 %out
+}
+
+define i8 @call_no_range() {
+; CHECK-LABEL: @call_no_range
+; CHECK-NEXT: %out = call i8 @dummy()
+; CHECK-NEXT: ret i8 %out
+ %out = call i8 @dummy()
+ ret i8 %out
+}
+
+define i8 @call_different_range() {
+; CHECK-LABEL: @call_different_range
+; CHECK-NEXT: %out = call range(i8 5, 7) i8 @dummy()
+; CHECK-NEXT: ret i8 %out
+ %out = call range(i8 5, 7) i8 @dummy()
+ ret i8 %out
+}
+
+define i8 @invoke_with_range() personality ptr undef {
+ %out = invoke range(i8 0, 2) i8 @dummy() to label %next unwind label %lpad
+
+next:
+ ret i8 %out
+
+lpad:
+ %pad = landingpad { ptr, i32 } cleanup
+ resume { ptr, i32 } zeroinitializer
+}
+
+define i8 @invoke_no_range() personality ptr undef {
+; CHECK-LABEL: @invoke_no_range()
+; CHECK-NEXT: invoke i8 @dummy
+ %out = invoke i8 @dummy() to label %next unwind label %lpad
+
+next:
+ ret i8 %out
+
+lpad:
+ %pad = landingpad { ptr, i32 } cleanup
+ resume { ptr, i32 } zeroinitializer
+}
+
+define i8 @invoke_different_range() personality ptr undef {
+; CHECK-LABEL: @invoke_different_range()
+; CHECK-NEXT: invoke range(i8 5, 7) i8 @dummy
+ %out = invoke range(i8 5, 7) i8 @dummy() to label %next unwind label %lpad
+
+next:
+ ret i8 %out
+
+lpad:
+ %pad = landingpad { ptr, i32 } cleanup
+ resume { ptr, i32 } zeroinitializer
+}
+
+define i8 @invoke_with_same_range() personality ptr undef {
+; CHECK-LABEL: @invoke_with_same_range()
+; CHECK: tail call i8 @invoke_with_range()
+ %out = invoke range(i8 0, 2) i8 @dummy() to label %next unwind label %lpad
+
+next:
+ ret i8 %out
+
+lpad:
+ %pad = landingpad { ptr, i32 } cleanup
+ resume { ptr, i32 } zeroinitializer
+}
+
+define i8 @call_with_same_range() {
+; CHECK-LABEL: @call_with_same_range
+; CHECK: tail call i8 @call_with_range
+ %out = call range(i8 0, 2) i8 @dummy()
+ ret i8 %out
+}
+
+define i8 @call_with_same_range_attr(i8 range(i8 0, 2) %v) {
+; CHECK-LABEL: @call_with_same_range_attr
+; CHECK: tail call i8 @call_with_range_attr
+ %out = call i8 @dummy2(i8 %v)
+ ret i8 %out
+}
+
+declare i8 @dummy();
+declare i8 @dummy2(i8);
+declare i32 @__gxx_personality_v0(...)
|
@@ -143,6 +143,14 @@ int FunctionComparator::cmpAttrs(const AttributeList L, | |||
if (int Res = cmpNumbers((uint64_t)TyL, (uint64_t)TyR)) | |||
return Res; | |||
continue; | |||
} else if (LA.isConstantRangeAttribute() && | |||
RA.isConstantRangeAttribute()) { |
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.
This still needs the getKindAsEnum check (in case there are more ConstantRange attributes in the future).
Might make sense to just move that check outside the isTypeAttribute branch and always perform it.
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.
nice catch. added the check. it needs to be inside the if branch as otherwise string attribute will not work
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
It is possible to update [AttributeImpl::operator<](https://github.com/andjo403/llvm-project/blob/a9da350aadfb5c86d36ae18398471558b22c1309/llvm/lib/IR/Attributes.cpp#L744) instead but feels strange to say that a range is less then an other range.
It is possible to update AttributeImpl::operator< instead but feels strange to say that a range is less then an other range.
CC @nikic