[CGP] Drop nneg flag when moving zext past instruction (#72103)

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.
llvm · Nov 14, 2023 · c9832da · c9832da
1 parent e96edde
commit c9832da
Show file tree

Hide file tree

Showing 5 changed files with 35 additions and 30 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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: