[PATCH] D142666: [DAGCombiner] Transform ABS(X) eq/ne 0/IntMin -> X eq/ne 0/IntMIn

[Noah Goldstein via Phabricator via llvm-commits]

goldstein.w.n added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:11698
+        if (N0->getNumOperands() == 2 && C->getAPIntValue().isMinSignedValue())
+          if (auto *C1 = dyn_cast<ConstantSDNode>(N0->getOperand(1)))
+            if (C1->getAPIntValue().isOne())
----------------
RKSimon wrote:
> Isn't ISD::ABS always one operand? We don't have the INT_MIN handling in the DAG node:
> ```
>   /// ABS - Determine the unsigned absolute value of a signed integer value of
>   /// the same bitwidth.
>   /// Note: A value of INT_MIN will return INT_MIN, no saturation or overflow
>   /// is performed.
>   ABS,
> ```
> Isn't ISD::ABS always one operand? We don't have the INT_MIN handling in the DAG node:
> ```
>   /// ABS - Determine the unsigned absolute value of a signed integer value of
>   /// the same bitwidth.
>   /// Note: A value of INT_MIN will return INT_MIN, no saturation or overflow
>   /// is performed.
>   ABS,
> ```

You're right, will fix if go forward with the patch, although based on the comment in `D142345` I think the consensus with this is truly unneeded so will likely abandon.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D142666/new/

https://reviews.llvm.org/D142666