Zext sext undef in sdag

[fengfeng09]

extend an undef value in sdag should keep the undef info.

[llvmbot]

@llvm/pr-subscribers-llvm-selectiondag

@llvm/pr-subscribers-backend-x86

Author: fengfeng (fengfeng09)

Changes

extend an undef value in sdag should keep the undef info.

---

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

2 Files Affected:

• (modified) llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp (+6-4)
• (modified) llvm/test/CodeGen/X86/zext-sext.ll (+10)

diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
index 203e14f6cde3e3..fe7d4b9d7ccbf9 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
@@ -6224,8 +6224,9 @@ SDValue SelectionDAG::getNode(unsigned Opcode, const SDLoc &DL, EVT VT,
       return getNode(OpOpcode, DL, VT, N1.getOperand(0), Flags);
     }
     if (OpOpcode == ISD::UNDEF)
-      // sext(undef) = 0, because the top bits will all be the same.
-      return getConstant(0, DL, VT);
+      // sext(undef) = undef in a conservative way, because not all of the bits
+      // are zero and there is no mechanism tracking the undef part.
+      return getUNDEF(VT);
     break;
   case ISD::ZERO_EXTEND:
     assert(VT.isInteger() && N1.getValueType().isInteger() &&
@@ -6244,8 +6245,9 @@ SDValue SelectionDAG::getNode(unsigned Opcode, const SDLoc &DL, EVT VT,
       return getNode(ISD::ZERO_EXTEND, DL, VT, N1.getOperand(0), Flags);
     }
     if (OpOpcode == ISD::UNDEF)
-      // zext(undef) = 0, because the top bits will be zero.
-      return getConstant(0, DL, VT);
+      // zext(undef) = undef in a conservative way, because not all of the bits
+      // are zero and there is no mechanism tracking the undef part.
+      return getUNDEF(VT);
 
     // Skip unnecessary zext_inreg pattern:
     // (zext (trunc x)) -> x iff the upper bits are known zero.
diff --git a/llvm/test/CodeGen/X86/zext-sext.ll b/llvm/test/CodeGen/X86/zext-sext.ll
index 25929ecbde76f1..766512afc2a7ec 100644
--- a/llvm/test/CodeGen/X86/zext-sext.ll
+++ b/llvm/test/CodeGen/X86/zext-sext.ll
@@ -77,5 +77,15 @@ entry:
   %alphaXbetaY = add i64 %alphaX, %tmp115
   %transformed = add i64 %alphaXbetaY, 9040145182981852475
   store i64 %transformed, ptr %d, align 8
+  %tmp200 = zext i16 undef to i32
+  %tmp201 = zext i16 undef to i32
+  %tmp202 = shl i32 %tmp201, 16
+  %tmp203 = or i32 %tmp200, %tmp202
+  store i32 %tmp203, ptr %a, align 4
+  %tmp210 = sext i16 undef to i32
+  %tmp211 = sext i16 undef to i32
+  %tmp212 = shl i32 %tmp211, 16
+  %tmp213 = or i32 %tmp210, %tmp212
+  store i32 %tmp213, ptr %b, align 4
   ret void
 }

[RKSimon]

What is the purpose behind this?

This approach is incorrect for how we handle extensions of undefs: https://alive2.llvm.org/ce/z/A4X7D5

[fengfeng09]

What is the purpose behind this?

This approach is incorrect for how we handle extensions of undefs: https://alive2.llvm.org/ce/z/A4X7D5

In fact, there is no practical significance for now. But it is counterintuitive to evaluate extend undef to a zero. Especially, when we use undef value as a element in BUILD_VECTOR, it may produce some unexpected zeros.

[arsenm]

This test doesn't show anything

[fengfeng09]

two 16 bit undef composed a zero？

[arsenm]

There are no checks for the output

[topperc]

This optimization is not correct for 'undef'. I think it would be correct for 'poison', but SelectionDAG doesn't have 'poison' today.

See also https://blog.regehr.org/archives/1837

[fengfeng09]

This optimization is not correct for 'undef'. I think it would be correct for 'poison', but SelectionDAG doesn't have 'poison' today.

See also https://blog.regehr.org/archives/1837

I see. Thanks for your explanation, so zext undef to a zero is by designed, is this right?

[topperc]

This optimization is not correct for 'undef'. I think it would be correct for 'poison', but SelectionDAG doesn't have 'poison' today.
See also https://blog.regehr.org/archives/1837

I see. Thanks for your explanation, so zext undef to a zero is by designed, is this right?

For (zext undef), the zero extended bits of the result must be 0. The bits that started out as undef can be any value. We choose 0 for simplicity.

For (sext undef), the sign extended bits must have the same value as the original sign bit. Because of undef, the original sign bit can be either 0 or 1. Folding to undef wouldn't guarantee the extended bits all match, so we fold to all 0s.

[fengfeng09]

This optimization is not correct for 'undef'. I think it would be correct for 'poison', but SelectionDAG doesn't have 'poison' today.
See also https://blog.regehr.org/archives/1837

I see. Thanks for your explanation, so zext undef to a zero is by designed, is this right?

For (zext undef), the zero extended bits of the result must be 0. The bits that started out as undef can be any value. We choose 0 for simplicity.

For (sext undef), the sign extended bits must have the same value as the original sign bit. Because of undef, the original sign bit can be either 0 or 1. Folding to undef wouldn't guarantee the extended bits all match, so we fold to all 0s.

Got it, thanks.

[fengfeng09]

This commit is not consistent with llvm undef refinement, so close it.