[llvm] d8f541e - [DAGCombiner] `visitFREEZE()`: fix handling of no maybe-poison ops

[Roman Lebedev via llvm-commits]

Author: Roman Lebedev
Date: 2022-12-23T17:26:05+03:00
New Revision: d8f541efe7b40fe49664054ad0e7306641762131

URL: https://github.com/llvm/llvm-project/commit/d8f541efe7b40fe49664054ad0e7306641762131
DIFF: https://github.com/llvm/llvm-project/commit/d8f541efe7b40fe49664054ad0e7306641762131.diff

LOG: [DAGCombiner] `visitFREEZE()`: fix handling of no maybe-poison ops

The original code was confusing. It was stripping poison-generating flags,
but the comments were saying that doing so was a TODO.

If the poison-generating flags are present, then even if all operands
are guaranteed not to be undef or poison, the whole operation may still
produce undef or poison. We can still deal with that case,
and we already do deal with it in fact, by also dropping those flags.

Refs. https://github.com/llvm/llvm-project/issues/59676

Added: 
    

Modified: 
    llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
    llvm/test/CodeGen/X86/freeze-binary.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
index 2568c2baf471..bf1f550c23c0 100644
--- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -14252,6 +14252,7 @@ SDValue DAGCombiner::visitFREEZE(SDNode *N) {
   // conditions 1) one-use, 2) does not produce poison, and 3) has all but one
   // guaranteed-non-poison operands (or is a BUILD_VECTOR or similar) then push
   // the freeze through to the operands that are not guaranteed non-poison.
+  // NOTE: we will strip poison-generating flags, so ignore them here.
   if (DAG.canCreateUndefOrPoison(N0, /*PoisonOnly*/ false,
                                  /*ConsiderFlags*/ false) ||
       N0->getNumValues() != 1 || !N0->hasOneUse())
@@ -14273,8 +14274,9 @@ SDValue DAGCombiner::visitFREEZE(SDNode *N) {
       return SDValue();
     }
   }
-  if (MaybePoisonOperands.empty())
-    return SDValue();
+  // NOTE: the whole op may be not guaranteed to not be undef or poison because
+  // it could create undef or poison due to it's poison-generating flags.
+  // So not finding any maybe-poison flags is fine.
 
   for (SDValue MaybePoisonOperand : MaybePoisonOperands) {
     // Don't replace every single UNDEF everywhere with frozen UNDEF, though.
@@ -14298,7 +14300,7 @@ SDValue DAGCombiner::visitFREEZE(SDNode *N) {
     if (Op.getOpcode() == ISD::UNDEF)
       Op = DAG.getFreeze(Op);
   }
-  // TODO: Just strip poison generating flags?
+  // NOTE: this strips poison generating flags.
   SDValue R = DAG.getNode(N0.getOpcode(), SDLoc(N0), N0->getVTList(), Ops);
   assert(DAG.isGuaranteedNotToBeUndefOrPoison(R, /*PoisonOnly*/ false) &&
          "Can't create node that may be undef/poison!");

diff  --git a/llvm/test/CodeGen/X86/freeze-binary.ll b/llvm/test/CodeGen/X86/freeze-binary.ll
index d3d4eac04a18..ea6e1fb36620 100644
--- a/llvm/test/CodeGen/X86/freeze-binary.ll
+++ b/llvm/test/CodeGen/X86/freeze-binary.ll
@@ -836,10 +836,9 @@ define void @pr59676_nsw_frozen(ptr %dst, i32 %x.orig) {
 ; X86-LABEL: pr59676_nsw_frozen:
 ; X86:       # %bb.0:
 ; X86-NEXT:    movl {{[0-9]+}}(%esp), %ecx
-; X86-NEXT:    movl {{[0-9]+}}(%esp), %edx
-; X86-NEXT:    imull $42, %edx, %eax
-; X86-NEXT:    imull %edx, %eax
-; X86-NEXT:    addl %eax, %eax
+; X86-NEXT:    movl {{[0-9]+}}(%esp), %eax
+; X86-NEXT:    imull %eax, %eax
+; X86-NEXT:    imull $84, %eax, %eax
 ; X86-NEXT:    movl $818089009, %edx # imm = 0x30C30C31
 ; X86-NEXT:    imull %edx
 ; X86-NEXT:    movl %edx, %eax
@@ -851,9 +850,8 @@ define void @pr59676_nsw_frozen(ptr %dst, i32 %x.orig) {
 ;
 ; X64-LABEL: pr59676_nsw_frozen:
 ; X64:       # %bb.0:
-; X64-NEXT:    imull $42, %esi, %eax
-; X64-NEXT:    imull %esi, %eax
-; X64-NEXT:    addl %eax, %eax
+; X64-NEXT:    imull %esi, %esi
+; X64-NEXT:    imull $84, %esi, %eax
 ; X64-NEXT:    cltq
 ; X64-NEXT:    imulq $818089009, %rax, %rax # imm = 0x30C30C31
 ; X64-NEXT:    movq %rax, %rcx
@@ -875,11 +873,10 @@ define void @pr59676_nsw_frozen(ptr %dst, i32 %x.orig) {
 define void @pr59676_nsw(ptr %dst, i32 %x) {
 ; X86-LABEL: pr59676_nsw:
 ; X86:       # %bb.0:
-; X86-NEXT:    movl {{[0-9]+}}(%esp), %edx
+; X86-NEXT:    movl {{[0-9]+}}(%esp), %eax
 ; X86-NEXT:    movl {{[0-9]+}}(%esp), %ecx
-; X86-NEXT:    imull $42, %edx, %eax
-; X86-NEXT:    imull %edx, %eax
-; X86-NEXT:    addl %eax, %eax
+; X86-NEXT:    imull %eax, %eax
+; X86-NEXT:    imull $84, %eax, %eax
 ; X86-NEXT:    movl $818089009, %edx # imm = 0x30C30C31
 ; X86-NEXT:    imull %edx
 ; X86-NEXT:    movl %edx, %eax
@@ -891,9 +888,8 @@ define void @pr59676_nsw(ptr %dst, i32 %x) {
 ;
 ; X64-LABEL: pr59676_nsw:
 ; X64:       # %bb.0:
-; X64-NEXT:    imull $42, %esi, %eax
-; X64-NEXT:    imull %esi, %eax
-; X64-NEXT:    addl %eax, %eax
+; X64-NEXT:    imull %esi, %esi
+; X64-NEXT:    imull $84, %esi, %eax
 ; X64-NEXT:    cltq
 ; X64-NEXT:    imulq $818089009, %rax, %rax # imm = 0x30C30C31
 ; X64-NEXT:    movq %rax, %rcx