[PATCH] D142601: [DAGCombiner]: Add transform (and/or (icmp eq/ne (A, C)), (icmp eq/ne (A, -C))) -> (icmp eq/ne (ABS A), ABS(C))

[Noah Goldstein via Phabricator via llvm-commits]

goldstein.w.n added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:5677
+    if (TargetPreference == AndOrSETCCFoldKind::ABS ||
+        DAG.doesNodeExist(ISD::ABS, DAG.getVTList(OpVT), {LHS0})) {
+      APInt C = LHS1C->getAPIntValue();
----------------
I don't think this is an issue, but it looks "close" to causing an infinite loop.
In `X86ISelLowering.cpp` from `D142345` there is a transform to covert scalar `(icmp eq/ne (ABS A), C)` -> the logicop/setcc pattern here that we are about to transform back to the `ABS` pattern. The `(icmp eq/ne (ABS A), C)` is guarded behind `hasOneUse` and this is guarded behind `doesNodeExist` so I believe its safe (after this transform the node will never have only one use), but whoever reviews should probably double check the reasoning/etc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142601