[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:
> dmgreen wrote:
> > 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.
> I did indeed. :) And despite that I still wondered if it was worth adding the extra argument at this point given there isn't any evidence that it's needed yet.
Hmm. I think it makes the function less generic, and it would be good to re-use it for ADDS/SUBS. I feel like that's just worse. But then I did rename the functions already (I didn't like the old name it had), and never found a good test case for the ADDS/SUBS which was a shame.


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

https://reviews.llvm.org/D118584