[PATCH] D74147: [AArch64] Add BIT/BIF support.

[Dave Green via Phabricator via llvm-commits]

dmgreen added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ExpandPseudoInsts.cpp:464-469
+        BuildMI(MBB, MBBI, MI.getDebugLoc(),
+                TII->get(Opcode == AArch64::BSLPv8i8 ? AArch64::ORRv8i8
+                                                     : AArch64::ORRv16i8))
+            .addReg(DstReg)
+            .add(MI.getOperand(1))
+            .add(MI.getOperand(1));
----------------
Do you know if there are any test cases that test this "extra move" path?


================
Comment at: llvm/lib/Target/AArch64/AArch64InstrFormats.td:5213
+  : Pseudo<(outs regtype:$dst), (ins regtype:$Rd, regtype:$Rn, regtype:$Rm), pattern>,
+    Sched<[WriteV]>;
+
----------------
ilinpv wrote:
> dmgreen wrote:
> > We should make sure that the schedules have info on BSLP, if they previously had explicit info for BSP/BIT/BIF.
> Schedules using explicit regex "^(BIF|BIT|BSL)". As I understand it schedules passes happen after pseudo expansion, so at schedule stage we have all BSLP expanded to BIF/BIT/BSL.
> 
I think we run scheduling both pre-ra and post-ra. But that might depend on the target. So the pre-ra scheduling will still see the BSLP's. (I think you are right about post-ra though, those will see the "real" instruction.)


================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.td:3968
+defm BIT : SIMDLogicalThreeVectorTied<1, 0b10, "bit", AArch64bit>;
+defm BIF : SIMDLogicalThreeVectorTied<1, 0b11, "bif", AArch64bslp>;
+
----------------
AArch64bslp can be null_frag here and in BSL.


================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.td:3988-4040
+def : Pat<(AArch64bslp (v8i8 V64:$Rd), V64:$Rn, V64:$Rm),
           (BSLv8i8 V64:$Rd, V64:$Rn, V64:$Rm)>;
-def : Pat<(AArch64bsl (v4i16 V64:$Rd), V64:$Rn, V64:$Rm),
+def : Pat<(AArch64bslp (v4i16 V64:$Rd), V64:$Rn, V64:$Rm),
           (BSLv8i8 V64:$Rd, V64:$Rn, V64:$Rm)>;
-def : Pat<(AArch64bsl (v2i32 V64:$Rd), V64:$Rn, V64:$Rm),
+def : Pat<(AArch64bslp (v2i32 V64:$Rd), V64:$Rn, V64:$Rm),
           (BSLv8i8 V64:$Rd, V64:$Rn, V64:$Rm)>;
+def : Pat<(AArch64bslp (v1i64 V64:$Rd), V64:$Rn, V64:$Rm),
----------------
Most of these patterns can be removed I think.  We need patterns for
`TriOpFrag<(or (and ...` -> BLSP (which are handled in the SIMDLogicalThreeVectorPseudo definnition)
AArch64bslp -> BSLP (for which we need these extra patterns).
AArch64bit -> BIT, if we are keeping that one around (again handled inside SIMDLogicalThreeVectorTied)

The other shouldn't be needed unless I'm missing something.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74147