llvm-reduce: Change function return types if function is not called

[arsenm]

Extend the early return on value reduction to mutate the function return
type if the function has no call uses. This could be generalized to rewrite
cases where all callsites are used, but it turns out that complicates the
visitation order given we try to compute all opportunities up front.

This is enough to cleanup the common case where we end up with one
function with a return of an uninteresting constant.

[arsenm]

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

• llvm-reduce: Support exotic terminators in instructions-to-return #134794
• llvm-reduce: Change function return types if function is not called #134035 👈 (View in Graphite)
• llvm-reduce: Reduce with early return of arguments #133627
• llvm-reduce: Add values to return reduction #132686
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[llvmbot]

@llvm/pr-subscribers-llvm-ir

Author: Matt Arsenault (arsenm)

Changes

Extend the early return on value reduction to mutate the function return
type if the function has no call uses. This could be generalized to rewrite
cases where all callsites are used, but it turns out that complicates the
visitation order given we try to compute all opportunities up front.

This is enough to cleanup the common case where we end up with one
function with a return of an uninteresting constant.

---

Full diff: https://github.com/llvm/llvm-project/pull/134035.diff

2 Files Affected:

• (added) llvm/test/tools/llvm-reduce/reduce-values-to-return-new-return-type.ll (+95)
• (modified) llvm/tools/llvm-reduce/deltas/ReduceValuesToReturn.cpp (+4-3)

diff --git a/llvm/test/tools/llvm-reduce/reduce-values-to-return-new-return-type.ll b/llvm/test/tools/llvm-reduce/reduce-values-to-return-new-return-type.ll
new file mode 100644
index 0000000000000..9ddbbe3def44f
--- /dev/null
+++ b/llvm/test/tools/llvm-reduce/reduce-values-to-return-new-return-type.ll
@@ -0,0 +1,95 @@
+; Test that llvm-reduce can move intermediate values by inserting
+; early returns when the function already has a different return type
+;
+; RUN: llvm-reduce --abort-on-invalid-reduction --delta-passes=instructions-to-return --test FileCheck --test-arg --check-prefix=INTERESTING --test-arg %s --test-arg --input-file %s -o %t
+; RUN: FileCheck --check-prefix=RESULT %s < %t
+
+
+@gv = global i32 0, align 4
+@ptr_array = global [2 x ptr] [ptr @inst_to_return_has_different_type_but_no_func_call_use,
+                               ptr @multiple_callsites_wrong_return_type]
+
+; Should rewrite this return from i64 to i32 since the function has no
+; uses.
+; INTERESTING-LABEL: @inst_to_return_has_different_type_but_no_func_call_use(
+; RESULT-LABEL: define i32 @inst_to_return_has_different_type_but_no_func_call_use(ptr %arg) {
+; RESULT-NEXT: %load = load i32, ptr %arg, align 4
+; RESULT-NEXT: ret i32 %load
+define i64 @inst_to_return_has_different_type_but_no_func_call_use(ptr %arg) {
+  %load = load i32, ptr %arg
+  store i32 %load, ptr @gv
+  ret i64 0
+}
+
+; INTERESTING-LABEL: @callsite_different_type_unused_0(
+; RESULT-LABEL: define i64 @inst_to_return_has_different_type_but_call_result_unused(
+; RESULT-NEXT: %load = load i32, ptr %arg
+; RESULT-NEXT: store i32 %load, ptr @gv
+; RESULT-NEXT: ret i64 0
+define void @callsite_different_type_unused_0(ptr %arg) {
+  %unused0 = call i64 @inst_to_return_has_different_type_but_call_result_unused(ptr %arg)
+  %unused1 = call i64 @inst_to_return_has_different_type_but_call_result_unused(ptr null)
+  ret void
+}
+
+; TODO: Could rewrite this return from i64 to i32 since the callsite is unused.
+; INTERESTING-LABEL: @inst_to_return_has_different_type_but_call_result_unused(
+; RESULT-LABEL: define i64 @inst_to_return_has_different_type_but_call_result_unused(
+; RESULT: ret i64 0
+define i64 @inst_to_return_has_different_type_but_call_result_unused(ptr %arg) {
+  %load = load i32, ptr %arg
+  store i32 %load, ptr @gv
+  ret i64 0
+}
+
+; INTERESTING-LABEL: @multiple_callsites_wrong_return_type(
+; RESULT-LABEL: define i64 @multiple_callsites_wrong_return_type(
+; RESULT: ret i64 0
+define i64 @multiple_callsites_wrong_return_type(ptr %arg) {
+  %load = load i32, ptr %arg
+  store i32 %load, ptr @gv
+  ret i64 0
+}
+
+; INTERESTING-LABEL: @unused_with_wrong_return_types(
+; RESULT-LABEL: define i64 @unused_with_wrong_return_types(
+; RESULT-NEXT: %unused0 = call i64 @multiple_callsites_wrong_return_type(ptr %arg)
+; RESULT-NEXT: ret i64 %unused0
+define void @unused_with_wrong_return_types(ptr %arg) {
+  %unused0 = call i64 @multiple_callsites_wrong_return_type(ptr %arg)
+  %unused1 = call i32 @multiple_callsites_wrong_return_type(ptr %arg)
+  %unused2 = call ptr @multiple_callsites_wrong_return_type(ptr %arg)
+  ret void
+}
+
+; INTERESTING-LABEL: @multiple_returns_wrong_return_type(
+; INTERESTING: %load0 = load i32,
+
+; RESULT-LABEL: define i32 @multiple_returns_wrong_return_type(
+; RESULT: ret i32
+; RESULT: ret i32
+; RESULT: ret i32
+define i32 @multiple_returns_wrong_return_type(ptr %arg, i1 %cond, i32 %arg2) {
+entry:
+  br i1 %cond, label %bb0, label %bb1
+
+bb0:
+  %load0 = load i32, ptr %arg
+  store i32 %load0, ptr @gv
+  ret i32 234
+
+bb1:
+  ret i32 %arg2
+
+bb2:
+  ret i32 34
+}
+
+; INTERESTING-LABEL: @call_multiple_returns_wrong_return_type(
+; RESULT-LABEL: define <2 x i32> @call_multiple_returns_wrong_return_type(
+; RESULT-NEXT: %unused = call <2 x i32> @multiple_returns_wrong_return_type(
+; RESULT-NEXT: ret <2 x i32> %unused
+define void @call_multiple_returns_wrong_return_type(ptr %arg, i1 %cond, i32 %arg2) {
+  %unused = call <2 x i32> @multiple_returns_wrong_return_type(ptr %arg, i1 %cond, i32 %arg2)
+  ret void
+}
diff --git a/llvm/tools/llvm-reduce/deltas/ReduceValuesToReturn.cpp b/llvm/tools/llvm-reduce/deltas/ReduceValuesToReturn.cpp
index 3e400ffc89482..0a02298acc73e 100644
--- a/llvm/tools/llvm-reduce/deltas/ReduceValuesToReturn.cpp
+++ b/llvm/tools/llvm-reduce/deltas/ReduceValuesToReturn.cpp
@@ -164,7 +164,9 @@ static bool canReplaceFuncUsers(const Function &F, Type *NewRetTy) {
     if (CB->getType() == NewRetTy)
       continue;
 
-    LLVM_DEBUG(dbgs() << "Cannot replace callsite with wrong type: " << *CB
+    // TODO: If all callsites have no uses, we could mutate the type of all the
+    // callsites. This will complicate the visit and rewrite ordering though.
+    LLVM_DEBUG(dbgs() << "Cannot replace used callsite with wrong type: " << *CB
                       << '\n');
     return false;
   }
@@ -200,8 +202,7 @@ static bool shouldForwardValueToReturn(const BasicBlock &BB, const Value *V,
   if (!isReallyValidReturnType(V->getType()))
     return false;
 
-  return (RetTy->isVoidTy() ||
-          (RetTy == V->getType() && shouldReplaceNonVoidReturnValue(BB, V))) &&
+  return (RetTy->isVoidTy() || shouldReplaceNonVoidReturnValue(BB, V)) &&
          canReplaceFuncUsers(*BB.getParent(), V->getType());
 }
 

[arsenm]

Forgot the part to fix the multiple return case

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[regehr]

LGTM!

[fhahn]

LGTM, thanks

[fhahn]

Comment needs updating?

Suggested change

	// Hack up any return values in other blocks, we can't leave them as ret void.
	// Hack up any return values in other blocks, we can't leave them as returning OldRetTy.