[llvm] [InstCombine] Add an option to skip simplification on call instruction where a non-void return value is expected while the callee returns void (PR #98536)

via llvm-commits llvm-commits at lists.llvm.org
Wed Jul 31 17:51:04 PDT 2024


https://github.com/yozhu updated https://github.com/llvm/llvm-project/pull/98536

>From 03bb6f3b5d309c9dd9f0c901ca006783ca98bfca Mon Sep 17 00:00:00 2001
From: YongKang Zhu <yongzhu at fb.com>
Date: Thu, 11 Jul 2024 13:20:54 -0700
Subject: [PATCH 1/3] [InstCombine] Add an option to not take advantage of void
 to non-void conversion on return value

---
 llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
index 0ca56f08c333d..880932159f570 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
@@ -89,6 +89,11 @@ static cl::opt<unsigned> GuardWideningWindow(
     cl::desc("How wide an instruction window to bypass looking for "
              "another guard"));
 
+static cl::opt<bool> SkipRetTypeVoidToNonVoidCallInst(
+    "instcombine-skip-call-ret-type-void-to-nonvoid", cl::init(false),
+    cl::desc("skip simplifying call instruction which expects a non-void "
+             "return value but the callee returns void"));
+
 /// Return the specified type promoted as it would be to pass though a va_arg
 /// area.
 static Type *getPromotedType(Type *Ty) {
@@ -4073,7 +4078,7 @@ bool InstCombinerImpl::transformConstExprCastCall(CallBase &Call) {
 
       if (!Caller->use_empty() &&
           // void -> non-void is handled specially
-          !NewRetTy->isVoidTy())
+          (SkipRetTypeVoidToNonVoidCallInst || !NewRetTy->isVoidTy()))
         return false;   // Cannot transform this return value.
     }
 

>From 0ac5dd82380eec075492aef8c160b51843a25c4f Mon Sep 17 00:00:00 2001
From: YongKang Zhu <yongzhu at fb.com>
Date: Mon, 15 Jul 2024 14:43:33 -0700
Subject: [PATCH 2/3] Add a test case

---
 .../skip-opt-void-to-non-void-conversion.ll   | 67 +++++++++++++++++++
 1 file changed, 67 insertions(+)
 create mode 100644 llvm/test/Transforms/InstCombine/skip-opt-void-to-non-void-conversion.ll

diff --git a/llvm/test/Transforms/InstCombine/skip-opt-void-to-non-void-conversion.ll b/llvm/test/Transforms/InstCombine/skip-opt-void-to-non-void-conversion.ll
new file mode 100644
index 0000000000000..f0d5cc2dd8d03
--- /dev/null
+++ b/llvm/test/Transforms/InstCombine/skip-opt-void-to-non-void-conversion.ll
@@ -0,0 +1,67 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; RUN: llvm-as %s -o %t.bc
+; RUN: opt --passes=instcombine %t.bc -S | FileCheck %s --check-prefix=OPT
+; RUN: opt --passes=instcombine --instcombine-skip-call-ret-type-void-to-nonvoid %t.bc -S | \
+; RUN:   FileCheck %s --check-prefix=SKIP
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-none-linux-android21"
+
+define dso_local void @foo() {
+entry:
+  ret void
+}
+
+define dso_local i32 @bar() {
+; OPT-LABEL: define dso_local i32 @bar() {
+; OPT-NEXT:  [[ENTRY:.*:]]
+; OPT-NEXT:    [[_RC_:%.*]] = alloca i32, align 4
+; OPT-NEXT:    [[TMP0:%.*]] = load i32, ptr [[_RC_]], align 4
+; OPT-NEXT:    [[CMP_NOT:%.*]] = icmp eq i32 [[TMP0]], 0
+; OPT-NEXT:    br i1 [[CMP_NOT]], label %[[IF_THEN:.*]], label %[[IF_END:.*]]
+; OPT:       [[IF_THEN]]:
+; OPT-NEXT:    tail call void @foo()
+; OPT-NEXT:    br i1 poison, label %[[IF_END]], label %[[CLEANUP:.*]]
+; OPT:       [[IF_END]]:
+; OPT-NEXT:    store i32 0, ptr [[_RC_]], align 4
+; OPT-NEXT:    br label %[[CLEANUP]]
+; OPT:       [[CLEANUP]]:
+; OPT-NEXT:    [[TMP1:%.*]] = load i32, ptr [[_RC_]], align 4
+; OPT-NEXT:    ret i32 [[TMP1]]
+;
+; SKIP-LABEL: define dso_local i32 @bar() {
+; SKIP-NEXT:  [[ENTRY:.*:]]
+; SKIP-NEXT:    [[_RC_:%.*]] = alloca i32, align 4
+; SKIP-NEXT:    [[TMP0:%.*]] = load i32, ptr [[_RC_]], align 4
+; SKIP-NEXT:    [[CMP_NOT:%.*]] = icmp eq i32 [[TMP0]], 0
+; SKIP-NEXT:    br i1 [[CMP_NOT]], label %[[IF_THEN:.*]], label %[[IF_END:.*]]
+; SKIP:       [[IF_THEN]]:
+; SKIP-NEXT:    [[CALL:%.*]] = tail call i32 @foo()
+; SKIP-NEXT:    [[CMP:%.*]] = icmp eq i32 [[CALL]], 0
+; SKIP-NEXT:    br i1 [[CMP]], label %[[IF_END]], label %[[CLEANUP:.*]]
+; SKIP:       [[IF_END]]:
+; SKIP-NEXT:    store i32 0, ptr [[_RC_]], align 4
+; SKIP-NEXT:    br label %[[CLEANUP]]
+; SKIP:       [[CLEANUP]]:
+; SKIP-NEXT:    [[TMP1:%.*]] = load i32, ptr [[_RC_]], align 4
+; SKIP-NEXT:    ret i32 [[TMP1]]
+;
+entry:
+  %_rc_ = alloca i32, align 4
+  %2 = load i32, ptr %_rc_, align 4
+  %cmp.not = icmp eq i32 %2, 0
+  br i1 %cmp.not, label %if.then, label %if.end
+
+if.then:
+  %call = tail call i32 @foo()
+  %cmp = icmp eq i32 %call, 0
+  br i1 %cmp, label %if.end, label %cleanup
+
+if.end:
+  store i32 0, ptr %_rc_, align 4
+  br label %cleanup
+
+cleanup:
+  %7 = load i32, ptr %_rc_, align 4
+  ret i32 %7
+}

>From 2a4d481b94861e4f6c771de54fc9c01ce1204a2c Mon Sep 17 00:00:00 2001
From: YongKang Zhu <yongzhu at fb.com>
Date: Wed, 31 Jul 2024 17:50:09 -0700
Subject: [PATCH 3/3] [instcombine] Remove transformation involving return
 value from call need void to non-void conversion

---
 .../InstCombine/InstCombineCalls.cpp          | 31 ++++-------
 .../skip-opt-void-to-non-void-conversion.ll   | 52 ++++++-------------
 2 files changed, 26 insertions(+), 57 deletions(-)

diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
index 880932159f570..493ca21c03d32 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
@@ -89,11 +89,6 @@ static cl::opt<unsigned> GuardWideningWindow(
     cl::desc("How wide an instruction window to bypass looking for "
              "another guard"));
 
-static cl::opt<bool> SkipRetTypeVoidToNonVoidCallInst(
-    "instcombine-skip-call-ret-type-void-to-nonvoid", cl::init(false),
-    cl::desc("skip simplifying call instruction which expects a non-void "
-             "return value but the callee returns void"));
-
 /// Return the specified type promoted as it would be to pass though a va_arg
 /// area.
 static Type *getPromotedType(Type *Ty) {
@@ -4076,9 +4071,7 @@ bool InstCombinerImpl::transformConstExprCastCall(CallBase &Call) {
       if (Callee->isDeclaration())
         return false;   // Cannot transform this return value.
 
-      if (!Caller->use_empty() &&
-          // void -> non-void is handled specially
-          (SkipRetTypeVoidToNonVoidCallInst || !NewRetTy->isVoidTy()))
+      if (!Caller->use_empty() || NewRetTy->isVoidTy())
         return false;   // Cannot transform this return value.
     }
 
@@ -4238,9 +4231,6 @@ bool InstCombinerImpl::transformConstExprCastCall(CallBase &Call) {
 
   AttributeSet FnAttrs = CallerPAL.getFnAttrs();
 
-  if (NewRetTy->isVoidTy())
-    Caller->setName("");   // Void type should not have a name.
-
   assert((ArgAttrs.size() == FT->getNumParams() || FT->isVarArg()) &&
          "missing argument attributes");
   AttributeList NewCallerPAL = AttributeList::get(
@@ -4269,17 +4259,14 @@ bool InstCombinerImpl::transformConstExprCastCall(CallBase &Call) {
   Instruction *NC = NewCall;
   Value *NV = NC;
   if (OldRetTy != NV->getType() && !Caller->use_empty()) {
-    if (!NV->getType()->isVoidTy()) {
-      NV = NC = CastInst::CreateBitOrPointerCast(NC, OldRetTy);
-      NC->setDebugLoc(Caller->getDebugLoc());
-
-      auto OptInsertPt = NewCall->getInsertionPointAfterDef();
-      assert(OptInsertPt && "No place to insert cast");
-      InsertNewInstBefore(NC, *OptInsertPt);
-      Worklist.pushUsersToWorkList(*Caller);
-    } else {
-      NV = PoisonValue::get(Caller->getType());
-    }
+    assert(!NV->getType()->isVoidTy());
+    NV = NC = CastInst::CreateBitOrPointerCast(NC, OldRetTy);
+    NC->setDebugLoc(Caller->getDebugLoc());
+
+    auto OptInsertPt = NewCall->getInsertionPointAfterDef();
+    assert(OptInsertPt && "No place to insert cast");
+    InsertNewInstBefore(NC, *OptInsertPt);
+    Worklist.pushUsersToWorkList(*Caller);
   }
 
   if (!Caller->use_empty())
diff --git a/llvm/test/Transforms/InstCombine/skip-opt-void-to-non-void-conversion.ll b/llvm/test/Transforms/InstCombine/skip-opt-void-to-non-void-conversion.ll
index f0d5cc2dd8d03..9d31f1e480f92 100644
--- a/llvm/test/Transforms/InstCombine/skip-opt-void-to-non-void-conversion.ll
+++ b/llvm/test/Transforms/InstCombine/skip-opt-void-to-non-void-conversion.ll
@@ -1,8 +1,6 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
 ; RUN: llvm-as %s -o %t.bc
-; RUN: opt --passes=instcombine %t.bc -S | FileCheck %s --check-prefix=OPT
-; RUN: opt --passes=instcombine --instcombine-skip-call-ret-type-void-to-nonvoid %t.bc -S | \
-; RUN:   FileCheck %s --check-prefix=SKIP
+; RUN: opt --passes=instcombine %t.bc -S | FileCheck %s
 
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-none-linux-android21"
@@ -13,38 +11,22 @@ entry:
 }
 
 define dso_local i32 @bar() {
-; OPT-LABEL: define dso_local i32 @bar() {
-; OPT-NEXT:  [[ENTRY:.*:]]
-; OPT-NEXT:    [[_RC_:%.*]] = alloca i32, align 4
-; OPT-NEXT:    [[TMP0:%.*]] = load i32, ptr [[_RC_]], align 4
-; OPT-NEXT:    [[CMP_NOT:%.*]] = icmp eq i32 [[TMP0]], 0
-; OPT-NEXT:    br i1 [[CMP_NOT]], label %[[IF_THEN:.*]], label %[[IF_END:.*]]
-; OPT:       [[IF_THEN]]:
-; OPT-NEXT:    tail call void @foo()
-; OPT-NEXT:    br i1 poison, label %[[IF_END]], label %[[CLEANUP:.*]]
-; OPT:       [[IF_END]]:
-; OPT-NEXT:    store i32 0, ptr [[_RC_]], align 4
-; OPT-NEXT:    br label %[[CLEANUP]]
-; OPT:       [[CLEANUP]]:
-; OPT-NEXT:    [[TMP1:%.*]] = load i32, ptr [[_RC_]], align 4
-; OPT-NEXT:    ret i32 [[TMP1]]
-;
-; SKIP-LABEL: define dso_local i32 @bar() {
-; SKIP-NEXT:  [[ENTRY:.*:]]
-; SKIP-NEXT:    [[_RC_:%.*]] = alloca i32, align 4
-; SKIP-NEXT:    [[TMP0:%.*]] = load i32, ptr [[_RC_]], align 4
-; SKIP-NEXT:    [[CMP_NOT:%.*]] = icmp eq i32 [[TMP0]], 0
-; SKIP-NEXT:    br i1 [[CMP_NOT]], label %[[IF_THEN:.*]], label %[[IF_END:.*]]
-; SKIP:       [[IF_THEN]]:
-; SKIP-NEXT:    [[CALL:%.*]] = tail call i32 @foo()
-; SKIP-NEXT:    [[CMP:%.*]] = icmp eq i32 [[CALL]], 0
-; SKIP-NEXT:    br i1 [[CMP]], label %[[IF_END]], label %[[CLEANUP:.*]]
-; SKIP:       [[IF_END]]:
-; SKIP-NEXT:    store i32 0, ptr [[_RC_]], align 4
-; SKIP-NEXT:    br label %[[CLEANUP]]
-; SKIP:       [[CLEANUP]]:
-; SKIP-NEXT:    [[TMP1:%.*]] = load i32, ptr [[_RC_]], align 4
-; SKIP-NEXT:    ret i32 [[TMP1]]
+; CHECK-LABEL: define dso_local i32 @bar() {
+; CHECK-NEXT:  [[ENTRY:.*:]]
+; CHECK-NEXT:    [[_RC_:%.*]] = alloca i32, align 4
+; CHECK-NEXT:    [[TMP0:%.*]] = load i32, ptr [[_RC_]], align 4
+; CHECK-NEXT:    [[CMP_NOT:%.*]] = icmp eq i32 [[TMP0]], 0
+; CHECK-NEXT:    br i1 [[CMP_NOT]], label %[[IF_THEN:.*]], label %[[IF_END:.*]]
+; CHECK:       [[IF_THEN]]:
+; CHECK-NEXT:    [[CALL:%.*]] = tail call i32 @foo()
+; CHECK-NEXT:    [[CMP:%.*]] = icmp eq i32 [[CALL]], 0
+; CHECK-NEXT:    br i1 [[CMP]], label %[[IF_END]], label %[[CLEANUP:.*]]
+; CHECK:       [[IF_END]]:
+; CHECK-NEXT:    store i32 0, ptr [[_RC_]], align 4
+; CHECK-NEXT:    br label %[[CLEANUP]]
+; CHECK:       [[CLEANUP]]:
+; CHECK-NEXT:    [[TMP1:%.*]] = load i32, ptr [[_RC_]], align 4
+; CHECK-NEXT:    ret i32 [[TMP1]]
 ;
 entry:
   %_rc_ = alloca i32, align 4



More information about the llvm-commits mailing list