[CGP] Drop nneg flag when moving zext past instruction

[nikic]

Fix the issue by not reusing the zext at all. The code already handles creation of new zexts if more than one is needed. Always use that code-path instead of trying to reuse the old zext in some case. (Alternatively we could also drop poison-generating flags on the old zext, but it seems cleaner to not reuse it at all, especially if it's not always possible anyway.)

Fixes #72046.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: Nikita Popov (nikic)

Changes

Fix the issue by not reusing the zext at all. The code already handles creation of new zexts if more than one is needed. Always use that code-path instead of trying to reuse the old zext in some case. (Alternatively we could also drop poison-generating flags on the old zext, but it seems cleaner to not reuse it at all, especially if it's not always possible anyway.)

Fixes #72046.

---

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

5 Files Affected:

• (modified) llvm/lib/CodeGen/CodeGenPrepare.cpp (+12-26)
• (modified) llvm/test/Transforms/CodeGenPrepare/X86/multi-extension.ll (+2-2)
• (added) llvm/test/Transforms/CodeGenPrepare/X86/pr72046.ll (+19)
• (modified) llvm/test/Transforms/CodeGenPrepare/X86/promoted-trunc-loc.ll (+1-1)
• (modified) llvm/test/Transforms/CodeGenPrepare/X86/promoted-zext-debugloc.ll (+1-1)

diff --git a/llvm/lib/CodeGen/CodeGenPrepare.cpp b/llvm/lib/CodeGen/CodeGenPrepare.cpp
index fa0c753bff2f073..07dc718ee3a38c3 100644
--- a/llvm/lib/CodeGen/CodeGenPrepare.cpp
+++ b/llvm/lib/CodeGen/CodeGenPrepare.cpp
@@ -4568,8 +4568,6 @@ Value *TypePromotionHelper::promoteOperandForOther(
   // Step #2.
   TPT.replaceAllUsesWith(Ext, ExtOpnd);
   // Step #3.
-  Instruction *ExtForOpnd = Ext;
-
   LLVM_DEBUG(dbgs() << "Propagate Ext to operands\n");
   for (int OpIdx = 0, EndOpIdx = ExtOpnd->getNumOperands(); OpIdx != EndOpIdx;
        ++OpIdx) {
@@ -4597,33 +4595,21 @@ Value *TypePromotionHelper::promoteOperandForOther(
     }
 
     // Otherwise we have to explicitly sign extend the operand.
-    // Check if Ext was reused to extend an operand.
-    if (!ExtForOpnd) {
-      // If yes, create a new one.
-      LLVM_DEBUG(dbgs() << "More operands to ext\n");
-      Value *ValForExtOpnd = IsSExt ? TPT.createSExt(Ext, Opnd, Ext->getType())
-                                    : TPT.createZExt(Ext, Opnd, Ext->getType());
-      if (!isa<Instruction>(ValForExtOpnd)) {
-        TPT.setOperand(ExtOpnd, OpIdx, ValForExtOpnd);
-        continue;
-      }
-      ExtForOpnd = cast<Instruction>(ValForExtOpnd);
-    }
+    Value *ValForExtOpnd = IsSExt
+                               ? TPT.createSExt(ExtOpnd, Opnd, Ext->getType())
+                               : TPT.createZExt(ExtOpnd, Opnd, Ext->getType());
+    TPT.setOperand(ExtOpnd, OpIdx, ValForExtOpnd);
+    Instruction *InstForExtOpnd = dyn_cast<Instruction>(ValForExtOpnd);
+    if (!InstForExtOpnd)
+      continue;
+
     if (Exts)
-      Exts->push_back(ExtForOpnd);
-    TPT.setOperand(ExtForOpnd, 0, Opnd);
+      Exts->push_back(InstForExtOpnd);
 
-    // Move the sign extension before the insertion point.
-    TPT.moveBefore(ExtForOpnd, ExtOpnd);
-    TPT.setOperand(ExtOpnd, OpIdx, ExtForOpnd);
-    CreatedInstsCost += !TLI.isExtFree(ExtForOpnd);
-    // If more sext are required, new instructions will have to be created.
-    ExtForOpnd = nullptr;
-  }
-  if (ExtForOpnd == Ext) {
-    LLVM_DEBUG(dbgs() << "Extension is useless now\n");
-    TPT.eraseInstruction(Ext);
+    CreatedInstsCost += !TLI.isExtFree(InstForExtOpnd);
   }
+  LLVM_DEBUG(dbgs() << "Extension is useless now\n");
+  TPT.eraseInstruction(Ext);
   return ExtOpnd;
 }
 
diff --git a/llvm/test/Transforms/CodeGenPrepare/X86/multi-extension.ll b/llvm/test/Transforms/CodeGenPrepare/X86/multi-extension.ll
index cc8e99176bf6e12..d09193e75f99742 100644
--- a/llvm/test/Transforms/CodeGenPrepare/X86/multi-extension.ll
+++ b/llvm/test/Transforms/CodeGenPrepare/X86/multi-extension.ll
@@ -10,8 +10,8 @@ declare void @bar(i64)
 ; %or is reachable by both a sext and zext that are going to be promoted.
 ; It ensures correct operation on PromotedInsts.
 
-; CHECK:       %promoted = trunc i32 %or to i16
-; CHECK-NEXT:  %c = sext i16 %promoted to i64
+; CHECK:       %promoted3 = trunc i32 %or to i16
+; CHECK-NEXT:  %c = sext i16 %promoted3 to i64
 define i32 @foo(i16 %kkk) {
 entry:
   %t4 = load i16, ptr @b, align 2
diff --git a/llvm/test/Transforms/CodeGenPrepare/X86/pr72046.ll b/llvm/test/Transforms/CodeGenPrepare/X86/pr72046.ll
new file mode 100644
index 000000000000000..d75e5632ebb2e58
--- /dev/null
+++ b/llvm/test/Transforms/CodeGenPrepare/X86/pr72046.ll
@@ -0,0 +1,19 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 3
+; RUN: opt -S -codegenprepare -mtriple=x86_64-unknown-unknown < %s | FileCheck %s
+
+; Make sure the nneg flag is dropped when lshr and zext are interchanged.
+define i8 @get(ptr %box, i32 %in) {
+; CHECK-LABEL: define i8 @get(
+; CHECK-SAME: ptr [[BOX:%.*]], i32 [[IN:%.*]]) {
+; CHECK-NEXT:    [[PROMOTED:%.*]] = zext i32 [[IN]] to i64
+; CHECK-NEXT:    [[SHR:%.*]] = lshr i64 [[PROMOTED]], 24
+; CHECK-NEXT:    [[ARRAYIDX:%.*]] = getelementptr inbounds i8, ptr [[BOX]], i64 [[SHR]]
+; CHECK-NEXT:    [[RES:%.*]] = load i8, ptr [[ARRAYIDX]], align 1
+; CHECK-NEXT:    ret i8 [[RES]]
+;
+  %shr = lshr i32 %in, 24
+  %idxprom = zext nneg i32 %shr to i64
+  %arrayidx = getelementptr inbounds i8, ptr %box, i64 %idxprom
+  %res = load i8, ptr %arrayidx, align 1
+  ret i8 %res
+}
diff --git a/llvm/test/Transforms/CodeGenPrepare/X86/promoted-trunc-loc.ll b/llvm/test/Transforms/CodeGenPrepare/X86/promoted-trunc-loc.ll
index 081c6742ede8c91..0ff35e1406de2bf 100644
--- a/llvm/test/Transforms/CodeGenPrepare/X86/promoted-trunc-loc.ll
+++ b/llvm/test/Transforms/CodeGenPrepare/X86/promoted-trunc-loc.ll
@@ -1,7 +1,7 @@
 ; RUN: opt < %s -codegenprepare -S -mtriple=x86_64-unknown-unknown | FileCheck %s --match-full-lines
 
 ; Make sure the promoted trunc doesn't get a debug location associated.
-; CHECK: %promoted = trunc i32 %or to i16
+; CHECK: %promoted3 = trunc i32 %or to i16
 
 target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-apple-macosx10.13.0"
diff --git a/llvm/test/Transforms/CodeGenPrepare/X86/promoted-zext-debugloc.ll b/llvm/test/Transforms/CodeGenPrepare/X86/promoted-zext-debugloc.ll
index 0b4f830ff3e7d99..f293d46ed6f74c6 100644
--- a/llvm/test/Transforms/CodeGenPrepare/X86/promoted-zext-debugloc.ll
+++ b/llvm/test/Transforms/CodeGenPrepare/X86/promoted-zext-debugloc.ll
@@ -1,7 +1,7 @@
 ; RUN: opt < %s -codegenprepare -S -mtriple=x86_64-unknown-unknown | FileCheck %s --match-full-lines
   
 ; Make sure the promoted zext doesn't get a debug location associated.
-; CHECK: %promoted = zext i8 %t to i64
+; CHECK: %promoted1 = zext i8 %t to i64
 
 define void @patatino(ptr %p, ptr %q, i32 %b, ptr %addr) !dbg !6 {
 entry:

[SixWeining]

LGTM but you can wait for someone else if you don't mind. Thanks.

[topperc]

LGTM

[heiher]

LGTM. Thanks!

[SixWeining]

This PR also fixes test-suite::2005-05-11-Popcount-ffs-fls.test https://lab.llvm.org/staging/#/builders/106/builds/1443.