[PATCH] D147266: [AArch64] Sink operands to allow for bitselect instructions

[Dave Green via Phabricator via cfe-commits]

dmgreen added a comment.

Thanks for working on this. I noticed there was another instance of vbsl being reported recently in https://github.com/llvm/llvm-project/issues/62642. Hopefully it can be addresses via extra optimizations too.

Can you add a testcase for the issues in https://github.com/llvm/llvm-project/issues/49305? And look into the existing tests.



================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:14343
+    // passed to this function. Starting pattern matching with any other
+    // instruction (such as Or) would lead to malformed IR
+
----------------
That sounds like it might be a bug that happens if it tries to sink too many operands? From what I remember the order they are put into Ops might matter. And if it is sinking to the Or it might need to add both the And as well as the Not.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:14346
+    // Check if this AND instruction is part of bigger tree rooted at Or.
+    if (Subtarget->hasNEON() && I->getNumUses() == 1) {
+      Use &U = *(I->use_begin());
----------------
I->hasOneUse()


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:14348
+      Use &U = *(I->use_begin());
+      Instruction *OI = cast<Instruction>(U.getUser());
+      Value *And0_Op0 = nullptr, *And0_Op1 = nullptr, *And1_Op0 = nullptr,
----------------
I->user_back();


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:14364
+        for (unsigned AndOpIndex = 0; AndOpIndex < 2; ++AndOpIndex) {
+          if (const Instruction *XI =
+                  dyn_cast<Instruction>(Ands[AndIndex][AndOpIndex]);
----------------
Can this be simplified with a m_Not matcher? In general instructions will be canonicalized so that constants are on the RHS.

If we are sinking the Not, I feel this should want to test that the pattern makes up a bsl, and if it does then sink the operand of I. i.e. something like checking that OI is `m_c_Or(m_c_And(m_Value(A),m_Value(B)),m_specific(I))`, and then checking that `I` is `m_c_And(m_Not(m_Specific(A)),m_Value(D))` or the other way around.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147266