[PATCH] D134946: [AArch64][SVE] Use flag setting variants of BRKB/BRKN/RDFFR

Mon Oct 3 07:57:59 PDT 2022

paulwalker-arm added inline comments.

================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.cpp:1335
+    auto *PredMask = MRI->getUniqueVRegDef(Pred->getOperand(1).getReg());
+    if (Mask != PredMask && (Mask != Pred))
+      return false;
----------------
bsmith wrote:
> Extra parenthesis
Are you sure this is universally true?  The `s` variants do an implicitly `ptest` using the original instruction's general predicate operand:
```
brkb p0.b, p1/z, p2.b; ptest p1, p0.b ==> brkbs p0.b, p1/z, p2.b
```
If you perform this transformation when the general predicates of both instructions differ the implicit ptest will calculate a result by considering a different set of lanes.

Whilst the logic might be safe for your `ptest.any` case because it's essentially an OR reduction (and thus safe when pg & data == data), the same is unlikely for things like cc.first and cc.none where it's critical to test the exact lanes requested.

Assuming you agree and I've not misunderstood you probably want to do something specific for `ptest.any`, which is best done during isel lowering.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134946