[PATCH] D101357: [CodeGen][AArch64][SVE] Substitute [rdffr, ptest] => rdffrs

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.cpp:1385-1388
+    case AArch64::RDFFR_P: {
+      // rdffr   p1.b               <--- Definition of Pred
+      // ptest   Mask=p0, Pred=p1.b <--- Can always optimize this to
+      //                                 rdffrs p1.b, p0/z
----------------
Is this true? I don't see how you know the value of `p0` just prior to `rdffr`.  There's nothing to say `p0` has been defined at this point nor that it contains the same value as it will by the time the `ptest` is hit.

Although not exactly the same this is what the
```
auto *RDFFRMask = MRI->getUniqueVRegDef(RDFFR->getOperand(1).getReg());
if (Mask != RDFFRMask)
  return false;
```
code for the `AArch64::RDFFR_PPz` case is protecting against.

Perhaps this case is worth a separate patch?


================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.cpp:1410-1412
+      PTest->eraseFromParent();
+      RDFFR->eraseFromParent();
+      return true;
----------------
Not saying there's anything wrong but I just wondered why you cannot fall through here?  Is the following code specific to the other case blocks? I only ask because `RDFFR_PPz->RDFFRS_PPz` looks very similar to `BRKN_PPzP->BRKNS_PPzP`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101357