Skip to content

[SelectionDAG] Remove redundant KnownBits smin and smax operations #89519

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 22, 2024

Conversation

AZero13
Copy link
Contributor

@AZero13 AZero13 commented Apr 20, 2024

It turns out that if any of the operations can be zero, and neither of the operands can be proven to be positive, it is possible for smax to be zero, and KnownBits cannot prove otherwise even with KnownBits::smax. In fact, proving it based on the KnownBits itself at that point without increasing the depth is actually, provably impossible.

Same with smin.

This covers all the possible cases and is proven to be complete.

@llvmbot llvmbot added the llvm:SelectionDAG SelectionDAGISel as well label Apr 20, 2024
@llvmbot
Copy link
Member

llvmbot commented Apr 20, 2024

@llvm/pr-subscribers-llvm-selectiondag

Author: AtariDreams (AtariDreams)

Changes

It turns out that if any of the operations can be zero, and neither of the operands can be proven to be positive, it is possible for smax to be zero, and KnownBits cannot prove otherwise even with KnownBits::smax. In fact, proving it based on the KnownBits itself at that point without increasing the depth is actually, provably impossible.

Same with smin.

This covers all the possible cases and is proven to be complete.


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

1 Files Affected:

  • (modified) llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp (-6)
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
index 7dbf83b7adeef0..b63b8b893fdbf1 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
@@ -5399,9 +5399,6 @@ bool SelectionDAG::isKnownNeverZero(SDValue Op, unsigned Depth) const {
     if (Op1.isNonZero() && Op0.isNonZero())
       return true;
 
-    if (KnownBits::smax(Op0, Op1).isNonZero())
-      return true;
-
     return isKnownNeverZero(Op.getOperand(1), Depth + 1) &&
            isKnownNeverZero(Op.getOperand(0), Depth + 1);
   }
@@ -5417,9 +5414,6 @@ bool SelectionDAG::isKnownNeverZero(SDValue Op, unsigned Depth) const {
     if (Op1.isNonZero() && Op0.isNonZero())
       return true;
 
-    if (KnownBits::smin(Op0, Op1).isNonZero())
-      return true;
-
     return isKnownNeverZero(Op.getOperand(1), Depth + 1) &&
            isKnownNeverZero(Op.getOperand(0), Depth + 1);
   }

@RKSimon RKSimon requested review from jayfoad and goldsteinn April 21, 2024 10:24
@AZero13
Copy link
Contributor Author

AZero13 commented Apr 21, 2024

@nikic

@AZero13
Copy link
Contributor Author

AZero13 commented Apr 21, 2024

@arsenm can you please merge this

It turns out that if any of the operations can be zero, and neither of the operands can be proven to be positive, it is possible for smax to be zero, and KnownBits cannot prove otherwise even with KnownBits::smax. In fact, proving it based on the KnownBits itself at that point without increasing the depth is actually, provably impossible.

Same with smin.

This covers all the possible cases and is proven to be complete.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
llvm:SelectionDAG SelectionDAGISel as well
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants