[DAG] Strip poison generating flags in freeze(op()) -> op(freeze()) fold
authorSimon Pilgrim <llvm-dev@redking.me.uk>
Fri, 26 Aug 2022 10:47:44 +0000 (11:47 +0100)
committerSimon Pilgrim <llvm-dev@redking.me.uk>
Fri, 26 Aug 2022 10:47:51 +0000 (11:47 +0100)
This patch follows the InstCombine approach of stripping poison generating flags (nsw/nuw from add/sub etc.) to allow us to push a freeze() through the op. Unlike InstCombine it doesn't retain any flags, but we have plenty of DAG folds that do the same thing already. We assert that the newly generated op isGuaranteedNotToBeUndefOrPoison.

Similar to the ValueTracking approach, isGuaranteedNotToBeUndefOrPoison has been updated to confirm that if an op can't create undef/poison and its operands are guaranteed not to be undef/poison - then its not undef/poison. This is just for the generic opcodes - target specific opcodes will need to do this manually just in case they have some special cases.

Differential Revision: https://reviews.llvm.org/D132333

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

index 1b699f8..032aa56 100644 (file)
@@ -13952,7 +13952,7 @@ SDValue DAGCombiner::visitFREEZE(SDNode *N) {
   // guaranteed-non-poison operands then push the freeze through to the one
   // operand that is not guaranteed non-poison.
   if (!DAG.canCreateUndefOrPoison(N0, /*PoisonOnly*/ false,
-                                  /*ConsiderFlags*/ true) &&
+                                  /*ConsiderFlags*/ false) &&
       N0->getNumValues() == 1 && N0->hasOneUse()) {
     SDValue MaybePoisonOperand;
     for (SDValue Op : N0->ops()) {
@@ -13969,7 +13969,6 @@ SDValue DAGCombiner::visitFREEZE(SDNode *N) {
     }
     if (MaybePoisonOperand) {
       // Recreate the node with the frozen maybe-poison operand.
-      // TODO: Disable ConsiderFlags and just strip poison generating flags?
       // TODO: Drop the isOnlyUserOf constraint and replace all users of
       // MaybePoisonOperand with FrozenMaybePoisonOperand
       // to match pushFreezeToPreventPoisonFromPropagating behavior.
@@ -13978,7 +13977,11 @@ SDValue DAGCombiner::visitFREEZE(SDNode *N) {
       for (SDValue &Op : Ops)
         if (Op == MaybePoisonOperand)
           Op = FrozenMaybePoisonOperand;
-      return DAG.getNode(N0.getOpcode(), SDLoc(N0), N0->getVTList(), Ops);
+      // TODO: Just strip 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!");
+      return R;
     }
   }
 
index d6688be..711bd92 100644 (file)
@@ -4551,9 +4551,9 @@ bool SelectionDAG::isGuaranteedNotToBeUndefOrPoison(SDValue Op,
     }
     return true;
 
-  // TODO: Search for noundef attributes from library functions.
+    // TODO: Search for noundef attributes from library functions.
 
-  // TODO: Pointers dereferenced by ISD::LOAD/STORE ops are noundef.
+    // TODO: Pointers dereferenced by ISD::LOAD/STORE ops are noundef.
 
   default:
     // Allow the target to implement this method for its nodes.
@@ -4564,7 +4564,15 @@ bool SelectionDAG::isGuaranteedNotToBeUndefOrPoison(SDValue Op,
     break;
   }
 
-  return false;
+  // If Op can't create undef/poison and none of its operands are undef/poison
+  // then Op is never undef/poison.
+  // NOTE: TargetNodes should handle this in themselves in
+  // isGuaranteedNotToBeUndefOrPoisonForTargetNode.
+  return !canCreateUndefOrPoison(Op, PoisonOnly, /*ConsiderFlags*/ true,
+                                 Depth) &&
+         all_of(Op->ops(), [&](SDValue V) {
+           return isGuaranteedNotToBeUndefOrPoison(V, PoisonOnly, Depth + 1);
+         });
 }
 
 bool SelectionDAG::canCreateUndefOrPoison(SDValue Op, bool PoisonOnly,
index 3d41927..e90cc31 100644 (file)
@@ -128,15 +128,13 @@ define i32 @freeze_add_nsw(i32 %a0) nounwind {
 ; X86-LABEL: freeze_add_nsw:
 ; X86:       # %bb.0:
 ; X86-NEXT:    movl {{[0-9]+}}(%esp), %eax
-; X86-NEXT:    incl %eax
-; X86-NEXT:    incl %eax
+; X86-NEXT:    addl $2, %eax
 ; X86-NEXT:    retl
 ;
 ; X64-LABEL: freeze_add_nsw:
 ; X64:       # %bb.0:
 ; X64-NEXT:    # kill: def $edi killed $edi def $rdi
-; X64-NEXT:    leal 1(%rdi), %eax
-; X64-NEXT:    incl %eax
+; X64-NEXT:    leal 2(%rdi), %eax
 ; X64-NEXT:    retq
   %x = add nsw i32 %a0, 1
   %y = freeze i32 %x
@@ -272,15 +270,15 @@ define i32 @freeze_mul_nsw(i32 %a0) nounwind {
 ; X86-LABEL: freeze_mul_nsw:
 ; X86:       # %bb.0:
 ; X86-NEXT:    movl {{[0-9]+}}(%esp), %eax
-; X86-NEXT:    leal (%eax,%eax,2), %eax
 ; X86-NEXT:    leal (%eax,%eax,4), %eax
+; X86-NEXT:    leal (%eax,%eax,2), %eax
 ; X86-NEXT:    retl
 ;
 ; X64-LABEL: freeze_mul_nsw:
 ; X64:       # %bb.0:
 ; X64-NEXT:    # kill: def $edi killed $edi def $rdi
-; X64-NEXT:    leal (%rdi,%rdi,2), %eax
-; X64-NEXT:    leal (%rax,%rax,4), %eax
+; X64-NEXT:    leal (%rdi,%rdi,4), %eax
+; X64-NEXT:    leal (%rax,%rax,2), %eax
 ; X64-NEXT:    retq
   %x = mul nsw i32 %a0, 3
   %y = freeze i32 %x
@@ -344,15 +342,13 @@ define i32 @freeze_shl_nsw(i32 %a0) nounwind {
 ; X86-LABEL: freeze_shl_nsw:
 ; X86:       # %bb.0:
 ; X86-NEXT:    movl {{[0-9]+}}(%esp), %eax
-; X86-NEXT:    shll $3, %eax
-; X86-NEXT:    shll $5, %eax
+; X86-NEXT:    shll $8, %eax
 ; X86-NEXT:    retl
 ;
 ; X64-LABEL: freeze_shl_nsw:
 ; X64:       # %bb.0:
-; X64-NEXT:    # kill: def $edi killed $edi def $rdi
-; X64-NEXT:    leal (,%rdi,8), %eax
-; X64-NEXT:    shll $5, %eax
+; X64-NEXT:    movl %edi, %eax
+; X64-NEXT:    shll $8, %eax
 ; X64-NEXT:    retq
   %x = shl nsw i32 %a0, 3
   %y = freeze i32 %x
@@ -398,8 +394,7 @@ define <2 x i64> @freeze_shl_vec(<2 x i64> %a0) nounwind {
 define <2 x i64> @freeze_shl_vec_outofrange(<2 x i64> %a0) nounwind {
 ; X86-LABEL: freeze_shl_vec_outofrange:
 ; X86:       # %bb.0:
-; X86-NEXT:    paddq %xmm0, %xmm0
-; X86-NEXT:    psllq $2, %xmm0
+; X86-NEXT:    psllq $3, %xmm0
 ; X86-NEXT:    retl
 ;
 ; X64-LABEL: freeze_shl_vec_outofrange: