[PATCH] D70253: [AArch64][SVE2] Implement remaining SVE2 floating-point intrinsics

Kerry McLaughlin via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Dec 2 03:18:08 PST 2019


kmclaughlin added inline comments.


================
Comment at: llvm/include/llvm/IR/IntrinsicsAArch64.td:898
+                 llvm_i32_ty],
+                [IntrNoMem]>;
+
----------------
sdesmalen wrote:
> efriedma wrote:
> > kmclaughlin wrote:
> > > sdesmalen wrote:
> > > > I'd expect the `llvm_i32_ty` to be an immediate for these instructions, right? If so you'll need to add `ImmArg<OpNo>`  to the list of properties.
> > > > 
> > > Thanks for taking a look at this :) I tried your suggestion of adding ImmAr<Op> to the list of properties here but had some problems with it (i.e. Cannot select: intrinsic %llvm.aarch64.sve.fmlalb.lane). I don't think this is too much of an issue here as we have additional checks on the immediate with VectorIndexH32b, which ensures the immediate is in the correct range.
> > The point of immarg markings isn't to assist the backend; it's to ensure IR optimizations don't break your intrinsic calls.
> The pattern is probably not matching because the immediate operand is a `TargetConstant` where the `AsmVectorIndexOpnd` derives from `ImmLeaf`, rather than `TImmLeaf` as introduced by D58232.
Thanks for the suggestion, this was the reason why the patterns were not matching! As this also affects many of the existing intrinsics not added here or in D70437, I would prefer to address this fully in a separate patch - do you have objections to this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70253





More information about the cfe-commits mailing list