Page MenuHomePhabricator

[AArch64][SVE2] Implement remaining SVE2 floating-point intrinsics
ClosedPublic

Authored by kmclaughlin on Nov 14 2019, 10:06 AM.

Details

Summary

Adds the following intrinsics:

  • faddp
  • fmaxp, fminp, fmaxnmp & fminnmp
  • fmlalb, fmlalt, fmlslb & fmlslt
  • flogb

Diff Detail

Event Timeline

kmclaughlin created this revision.Nov 14 2019, 10:06 AM
sdesmalen added inline comments.Nov 22 2019, 3:56 AM
llvm/include/llvm/IR/IntrinsicsAArch64.td
958

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.

kmclaughlin added inline comments.Nov 27 2019, 10:10 AM
llvm/include/llvm/IR/IntrinsicsAArch64.td
958

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.

efriedma added inline comments.
llvm/include/llvm/IR/IntrinsicsAArch64.td
958

The point of immarg markings isn't to assist the backend; it's to ensure IR optimizations don't break your intrinsic calls.

sdesmalen added inline comments.Nov 29 2019, 4:55 AM
llvm/include/llvm/IR/IntrinsicsAArch64.td
958

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.

kmclaughlin added inline comments.Dec 2 2019, 3:17 AM
llvm/include/llvm/IR/IntrinsicsAArch64.td
958

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?

sdesmalen accepted this revision.Dec 2 2019, 9:40 AM

Thanks @kmclaughlin , LGTM.

llvm/include/llvm/IR/IntrinsicsAArch64.td
958

Okay, I'm happy with you want to make that change in a separate patch. It will also be needed for several of the other SVE intrinsics.

This revision is now accepted and ready to land.Dec 2 2019, 9:40 AM
This revision was automatically updated to reflect the committed changes.