miyengar added inline comments.
================ Comment at: llvm/lib/Target/AArch64/AArch64InstrFormats.td:6309 + + def : Pat<(v1f64 (OpNode (v1f64 FPR64:$Rn))), + (!cast<Instruction>(NAME # Dr) FPR64:$Rn)>; ---------------- dmgreen wrote: > miyengar wrote: > > dmgreen wrote: > > > I think the instructions in this multiclass are the vector variants. Can > > > the pattern be moved to the FRIntNNT/SingleOperandFPNo16 class? > > Thanks for the comment. I have tried to do this, but have run into a > > problem: > > > > This pattern targets the LLVM internal intrinsic: > > `int_aarch64_neon_frint***_v1f64`, as referenced in `AArch64InstrInfo.td`: > > > > ``` > > defm FRINT32Z : FRIntNNTVector<0, 0, "frint32z", int_aarch64_neon_frint32z>; > > ``` > > It pattern matches 'v1f64' to 'Dr', which corresponds to a scalar > > instruction defined in FRIntNNT/SingleOperandFPNo16 - I.e. it is matching > > the vector variant to the scalar variant. > > > > Moving the pattern to FRIntNNT results in an error "Cannot select: > > intrinsic %llvm.aarch64.neon.frint64x" as FRIntNNT is designed to match the > > intrinsics whose name does not contain "neon". Since the current patch > > deals specifically with the neon variant, I don't see an uncomplicated way > > to modify FRIntNNT to accommodate both variants of such intrinsic (neon and > > vanilla). > > > > I have moved the pattern from SIMDTwoVectorSD to FRIntNNTVector though > > which is hopefully a better place to put it. > > > > Does this seem like a sensible solution? Thanks! > I see, Because there is a difference between int_aarch64_frint32z and > int_aarch64_neon_frint32z. > > Could you pass int_aarch64_neon_frint32z to FRIntNNT too, or make them free > patterns in AArch64InstrInfo.td? Otherwise this is using the class to > generate vector variants of FRINT32Z to also generate the scalar FRINT32ZDr > instruction too. It likely doesn't matter a huge amount, but it feels like it > breaks the encapsulation of the FRIntNNTVector class. Ah okay, this makes sense, thank you! I have moved the patterns from FRIntNNTVector to make them free patterns in AArch64InstrInfo.td instead - this should preserve the encapsulation of the FRIntNNTVector class. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158626/new/ https://reviews.llvm.org/D158626 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
