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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to