[PATCH] D118584: [AArch64] Combine ISD::AND into AArch64ISD::ANDS

[Dave Green via Phabricator via llvm-commits]

dmgreen added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:14084
+// cases.
+static SDValue performANDSCombine(SDNode *N, unsigned GenericOpc,
+                                  TargetLowering::DAGCombinerInfo &DCI) {
----------------
david-arm wrote:
> Is there any need to pass in `GenericOpc` as it seems to always be ISD::AND?
Did you see the Note 2 lines up :)

This code can also be used for AArch64::ADDS and AArch64::SUBS, I just couldn't find a testcase where it modified things yet. On X86 they lower certain x86 intrinsics straight to the nodes with make them easier to write tests for.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:14094
+    SDValue Res = DCI.DAG.getNode(GenericOpc, DL, VT, LHS, RHS);
+    return DCI.DAG.getMergeValues({Res, DCI.DAG.getConstant(0, DL, MVT::i32)},
+                                  DL);
----------------
david-arm wrote:
> Do you have to do this given we're not merging any results together? For example, does it work if you just do
> 
>   return DCI.DAG.getNode(GenericOpc, DL, VT, LHS, RHS);
> 
> ?
Hmm. A node that produces 2 results needs to be replaced by a node the produces 2 results.


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

https://reviews.llvm.org/D118584