[CGP] Drop poison-generating flags after hoisting by dtcxzyw · Pull Request #90382 · llvm/llvm-project

dtcxzyw · 2024-04-28T08:26:45Z

See the following case:

define i8 @src1(i8 %x) {
entry:
  %cmp = icmp eq i8 %x, -1
  br i1 %cmp, label %exit, label %if.then

if.then:
  %inc = add nuw nsw i8 %x, 1
  br label %exit

exit:
  %retval = phi i8 [ %inc, %if.then ], [ -1, %entry ]
  ret i8 %retval
}

define i8 @tgt1(i8 %x) {
entry:
  %inc = add nuw nsw i8 %x, 1
  %0 = icmp eq i8 %inc, 0
  br i1 %0, label %exit, label %if.then

if.then:                                          ; preds = %entry
  br label %exit

exit:                                             ; preds = %if.then, %entry
  %retval = phi i8 [ %inc, %if.then ], [ -1, %entry ]
  ret i8 %retval
}

optimizeBranch converts icmp eq X, -1 into cmp to zero on RISC-V and hoists the add into the entry block. Poison-generating flags should be dropped as they don't still hold.

Proof: https://alive2.llvm.org/ce/z/sP7mvK
Fixes #90380

llvmbot · 2024-04-28T08:27:17Z

@llvm/pr-subscribers-llvm-transforms

Author: Yingwei Zheng (dtcxzyw)

Changes

See the following case:

define i8 @<!-- -->src1(i8 %x) {
entry:
  %cmp = icmp eq i8 %x, -1
  br i1 %cmp, label %exit, label %if.then

if.then:
  %inc = add nuw nsw i8 %x, 1
  br label %exit

exit:
  %retval = phi i8 [ %inc, %if.then ], [ -1, %entry ]
  ret i8 %retval
}

define i8 @<!-- -->tgt1(i8 %x) {
entry:
  %inc = add nuw nsw i8 %x, 1
  %0 = icmp eq i8 %inc, 0
  br i1 %0, label %exit, label %if.then

if.then:                                          ; preds = %entry
  br label %exit

exit:                                             ; preds = %if.then, %entry
  %retval = phi i8 [ %inc, %if.then ], [ -1, %entry ]
  ret i8 %retval
}

optimizeBranch converts icmp eq X, -1 into cmp to zero on RISC-V and hoists the add into the entry block. Poison-generating flags should be dropped as they don't still hold.

Proof: https://alive2.llvm.org/ce/z/sP7mvK
Fixes #90380

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

2 Files Affected:

(modified) llvm/lib/CodeGen/CodeGenPrepare.cpp (+6-2)
(added) llvm/test/Transforms/CodeGenPrepare/RISCV/convert-to-eqz.ll (+54)

diff --git a/llvm/lib/CodeGen/CodeGenPrepare.cpp b/llvm/lib/CodeGen/CodeGenPrepare.cpp
index 8eaf78157550ee..9e9c0a09190f37 100644
--- a/llvm/lib/CodeGen/CodeGenPrepare.cpp
+++ b/llvm/lib/CodeGen/CodeGenPrepare.cpp
@@ -8268,8 +8268,10 @@ static bool optimizeBranch(BranchInst *Branch, const TargetLowering &TLI,
     if (CmpC.isPowerOf2() && Cmp->getPredicate() == ICmpInst::ICMP_ULT &&
         match(UI, m_Shr(m_Specific(X), m_SpecificInt(CmpC.logBase2())))) {
       IRBuilder<> Builder(Branch);
-      if (UI->getParent() != Branch->getParent())
+      if (UI->getParent() != Branch->getParent()) {
         UI->moveBefore(Branch);
+        UI->dropPoisonGeneratingFlags();
+      }
       Value *NewCmp = Builder.CreateCmp(ICmpInst::ICMP_EQ, UI,
                                         ConstantInt::get(UI->getType(), 0));
       LLVM_DEBUG(dbgs() << "Converting " << *Cmp << "\n");
@@ -8281,8 +8283,10 @@ static bool optimizeBranch(BranchInst *Branch, const TargetLowering &TLI,
         (match(UI, m_Add(m_Specific(X), m_SpecificInt(-CmpC))) ||
          match(UI, m_Sub(m_Specific(X), m_SpecificInt(CmpC))))) {
       IRBuilder<> Builder(Branch);
-      if (UI->getParent() != Branch->getParent())
+      if (UI->getParent() != Branch->getParent()) {
         UI->moveBefore(Branch);
+        UI->dropPoisonGeneratingFlags();
+      }
       Value *NewCmp = Builder.CreateCmp(Cmp->getPredicate(), UI,
                                         ConstantInt::get(UI->getType(), 0));
       LLVM_DEBUG(dbgs() << "Converting " << *Cmp << "\n");
diff --git a/llvm/test/Transforms/CodeGenPrepare/RISCV/convert-to-eqz.ll b/llvm/test/Transforms/CodeGenPrepare/RISCV/convert-to-eqz.ll
new file mode 100644
index 00000000000000..a08ca34f01a9f5
--- /dev/null
+++ b/llvm/test/Transforms/CodeGenPrepare/RISCV/convert-to-eqz.ll
@@ -0,0 +1,54 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 4
+; RUN: opt -codegenprepare -S -mtriple=riscv64 < %s | FileCheck %s
+
+define i8 @hoist_add(i8 %x) {
+; CHECK-LABEL: define i8 @hoist_add(
+; CHECK-SAME: i8 [[X:%.*]]) {
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[INC:%.*]] = add i8 [[X]], 1
+; CHECK-NEXT:    [[TMP0:%.*]] = icmp eq i8 [[INC]], 0
+; CHECK-NEXT:    br i1 [[TMP0]], label [[EXIT:%.*]], label [[IF_THEN:%.*]]
+; CHECK:       if.then:
+; CHECK-NEXT:    br label [[EXIT]]
+; CHECK:       exit:
+; CHECK-NEXT:    [[RETVAL:%.*]] = phi i8 [ [[INC]], [[IF_THEN]] ], [ -1, [[ENTRY:%.*]] ]
+; CHECK-NEXT:    ret i8 [[RETVAL]]
+;
+entry:
+  %cmp = icmp eq i8 %x, -1
+  br i1 %cmp, label %exit, label %if.then
+
+if.then:
+  %inc = add nuw nsw i8 %x, 1
+  br label %exit
+
+exit:
+  %retval = phi i8 [ %inc, %if.then ], [ -1, %entry ]
+  ret i8 %retval
+}
+
+define i8 @hoist_lshr(i8 %x) {
+; CHECK-LABEL: define i8 @hoist_lshr(
+; CHECK-SAME: i8 [[X:%.*]]) {
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[INC:%.*]] = lshr i8 [[X]], 3
+; CHECK-NEXT:    [[TMP0:%.*]] = icmp eq i8 [[INC]], 0
+; CHECK-NEXT:    br i1 [[TMP0]], label [[EXIT:%.*]], label [[IF_THEN:%.*]]
+; CHECK:       if.then:
+; CHECK-NEXT:    br label [[EXIT]]
+; CHECK:       exit:
+; CHECK-NEXT:    [[RETVAL:%.*]] = phi i8 [ [[INC]], [[IF_THEN]] ], [ -1, [[ENTRY:%.*]] ]
+; CHECK-NEXT:    ret i8 [[RETVAL]]
+;
+entry:
+  %cmp = icmp ult i8 %x, 8
+  br i1 %cmp, label %exit, label %if.then
+
+if.then:
+  %inc = lshr exact i8 %x, 3
+  br label %exit
+
+exit:
+  %retval = phi i8 [ %inc, %if.then ], [ -1, %entry ]
+  ret i8 %retval
+}

llvm/lib/CodeGen/CodeGenPrepare.cpp

antoniofrighetto · 2024-04-28T13:53:10Z

I just wonder if there are possibly other places in CGP where we should drop the flags, but I assume that, unless we have a test case showing that explicitly, it's fine as it is.

nikic

LGTM, but is it possible to add these tests in llvm/test/Transforms/CodeGenPrepare/ARM/branch-on-zero.ll, which has the other tests for this transform?

dtcxzyw · 2024-04-29T07:58:03Z

/cherry-pick ab12bba

See the following case: ``` define i8 @SRC1(i8 %x) { entry: %cmp = icmp eq i8 %x, -1 br i1 %cmp, label %exit, label %if.then if.then: %inc = add nuw nsw i8 %x, 1 br label %exit exit: %retval = phi i8 [ %inc, %if.then ], [ -1, %entry ] ret i8 %retval } define i8 @tgt1(i8 %x) { entry: %inc = add nuw nsw i8 %x, 1 %0 = icmp eq i8 %inc, 0 br i1 %0, label %exit, label %if.then if.then: ; preds = %entry br label %exit exit: ; preds = %if.then, %entry %retval = phi i8 [ %inc, %if.then ], [ -1, %entry ] ret i8 %retval } ``` `optimizeBranch` converts `icmp eq X, -1` into cmp to zero on RISC-V and hoists the add into the entry block. Poison-generating flags should be dropped as they don't still hold. Proof: https://alive2.llvm.org/ce/z/sP7mvK Fixes llvm#90380 (cherry picked from commit ab12bba)

llvmbot · 2024-04-29T08:03:10Z

/pull-request #90437

See the following case: ``` define i8 @SRC1(i8 %x) { entry: %cmp = icmp eq i8 %x, -1 br i1 %cmp, label %exit, label %if.then if.then: %inc = add nuw nsw i8 %x, 1 br label %exit exit: %retval = phi i8 [ %inc, %if.then ], [ -1, %entry ] ret i8 %retval } define i8 @tgt1(i8 %x) { entry: %inc = add nuw nsw i8 %x, 1 %0 = icmp eq i8 %inc, 0 br i1 %0, label %exit, label %if.then if.then: ; preds = %entry br label %exit exit: ; preds = %if.then, %entry %retval = phi i8 [ %inc, %if.then ], [ -1, %entry ] ret i8 %retval } ``` `optimizeBranch` converts `icmp eq X, -1` into cmp to zero on RISC-V and hoists the add into the entry block. Poison-generating flags should be dropped as they don't still hold. Proof: https://alive2.llvm.org/ce/z/sP7mvK Fixes llvm#90380 (cherry picked from commit ab12bba)

dtcxzyw requested review from RKSimon, nikic, preames and topperc April 28, 2024 08:26

llvmbot added the llvm:transforms label Apr 28, 2024

nikic reviewed Apr 28, 2024

View reviewed changes

llvm/lib/CodeGen/CodeGenPrepare.cpp Outdated Show resolved Hide resolved

dtcxzyw added 2 commits April 28, 2024 20:40

[CGP] Add pre-commit tests for PR90380. NFC.

1c8e4bb

[CGP] Drop poison-generating flags in optimizeBranch

7e7bf0b

dtcxzyw force-pushed the cgp-clean-pgf branch from fbd8047 to 7e7bf0b Compare April 28, 2024 13:11

nikic approved these changes Apr 29, 2024

View reviewed changes

[CGP] Add additional tests for ARM

e2130c4

dtcxzyw merged commit ab12bba into llvm:main Apr 29, 2024

dtcxzyw deleted the cgp-clean-pgf branch April 29, 2024 07:51

dtcxzyw added this to the LLVM 18.X Release milestone Apr 29, 2024

pointhex mentioned this pull request May 7, 2024

getStyleDiagHandler #91314

Closed

aemerson mentioned this pull request May 9, 2024

release/18.x: [AArc64][GlobalISel] Fix legalizer assert for G_INSERT_VECTOR_ELT - manual merge #91672

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CGP] Drop poison-generating flags after hoisting#90382

[CGP] Drop poison-generating flags after hoisting#90382
dtcxzyw merged 3 commits intollvm:mainfrom
dtcxzyw:cgp-clean-pgf

dtcxzyw commented Apr 28, 2024

Uh oh!

llvmbot commented Apr 28, 2024

Uh oh!

Uh oh!

antoniofrighetto commented Apr 28, 2024

Uh oh!

nikic left a comment

Uh oh!

dtcxzyw commented Apr 29, 2024

Uh oh!

llvmbot commented Apr 29, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

dtcxzyw commented Apr 28, 2024

Uh oh!

llvmbot commented Apr 28, 2024

Uh oh!

Uh oh!

antoniofrighetto commented Apr 28, 2024

Uh oh!

nikic left a comment

Choose a reason for hiding this comment

Uh oh!

dtcxzyw commented Apr 29, 2024

Uh oh!

llvmbot commented Apr 29, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants