[PATCH] D100304: [AArch64][NEON] Match (or (and -a b) (and (a+1) b)) => bit select

[Bradley Smith via Phabricator via llvm-commits]

bsmith added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:12581-12597
+      SDValue O0 = N0->getOperand(i);
+      SDValue O1 = N1->getOperand(j);
+      SDValue Sub, Add, SubSibling, AddSibling;
+
+      // Find a SUB and an ADD operand, one from each AND.
+      if (O0.getOpcode() == ISD::SUB && O1.getOpcode() == ISD::ADD) {
+        Sub = O0;
----------------
joechrisellis wrote:
> Small suggestion -- it might be possible to get rid of some duplicate code here by doing something like:
> 
> ```
> if (O0.getOpcode() == ISD::SUB)
>     std::swap(O0, O1);
> 
> if (O1.getOpcode() != ISD::ADD)
>     continue;
> 
> // if we get here, O0 is guaranteed to be the sub, and O1 the add
> ...
> ```
> 
> If this works, I _think_ it might make things clearer. But I don't know for sure. If it starts to look uglier then I have no problem with it as-is. :) 
To be honest I think something like this is just going to end up being more confusing, for the sake of saving a couple of duplicated lines, I really don't think it's worth it. Also note that your example doesn't work since the second check isn't correct in the case the operands have been swapped.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100304