Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Arm64] Implement more hardware intrinsics from #24794 #33889

Merged

Conversation

echesakov
Copy link
Contributor

  1. Implements the following intrinsics as proposed in API Proposal: More SIMD HW Intrinsics #24794:
  • AbsoluteDifferenceAdd
  • MultiplyExtended, MultiplyExtendedScalar
  • PolynomialMultiply
  • ReciprocalEstimate, ReciprocalEstimateScalar
  • ReciprocalExponentScalar
  • ReciprocalSquareRootEstimate, ReciprocalSquareRootEstimateScalar
  • ReciprocalSquareRootStep, ReciprocalSquareRootStepScalar
  • ReciprocalStep, ReciprocalStepScalar
  1. Implements frecpe, frecps, frecpx, frsqrte, frsqrts, urecpe, ursqrte instructions

  2. Fixes implementation for fcmeq, fcmge, fcmgt, fcmle, fcmlt (zero immediate) instructions

  3. Adds emitter unit tests for the new added instructions and the fixed ones.
    Attached are the outputs for jitDisasm and WinDbg u command

jitDisasm.txt
windbg-u.txt

  1. Adds missing flag BaseTypeFromFirstArg for AbsoluteDifference intrinsics

…ar in AdvSimd.cs AdvSimd.PlatformNotSupported.cs
@echesakov echesakov added arch-arm64 area-System.Runtime.Intrinsics area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI labels Mar 20, 2020
@Dotnet-GitSync-Bot
Copy link
Collaborator

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@echesakov
Copy link
Contributor Author

@echesakov echesakov marked this pull request as ready for review March 20, 2020 20:53
Copy link
Contributor

@CarolEidt CarolEidt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - thanks!

@echesakov
Copy link
Contributor Author

LGTM - thanks!

Thank you for review, Carol!

@CarolEidt @tannergooding One additional question:

As you probably noticed one of my last commits was 9e7b2f1 - updating lsraarm64.cpp to reflect that the instructions uaba and saba are RMW.

While that change is sound, ideally, I want this to be specified via the table in hwintrinsiclistarm64.h.
However, since most of the arm64 intrinsics are not RMW this would require marking most of them with HW_Flag_NoRMWSemantics.

I would like to avoid this and propose to invert this flag on arm64, i.e. have no-RMW semantic for intrinsics as default and mark a smaller subset of them with a new flag HW_Flag_HasRMWSemantics.

In other words, I would like to

  1. remove definition of HW_Flag_NoRMWSemantics flag on arm64
  2. add HW_Flag_HasRMWSemantics
  3. make the logic as in 9e7b2f1 to be table-driven.
    I will still need to keep some logic for tgtPrefUse though.

@CarolEidt @tannergooding Will you have any concerns with such change?

@tannergooding
Copy link
Member

I'm fine with the underlying flags differing, we have the helper functions that people should be calling to get that info anyways and so we shouldn't really have anything but the table (and those functions) using the flags directly.

@CarolEidt
Copy link
Contributor

I agree with Tanner - it makes sense to have the opposite default for Arm64

@echesakov echesakov merged commit eff3797 into dotnet:master Mar 24, 2020
@echesakov echesakov deleted the Arm64-More-Hardware-Intrinsics-GitHub-24794 branch March 24, 2020 02:39
@ghost ghost locked as resolved and limited conversation to collaborators Dec 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-arm64 area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI area-System.Runtime.Intrinsics new-api-needs-documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants