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] Support table-driven code generation for scalar intrinsics #208

Closed
wants to merge 1 commit into from
Closed

[Arm64] Support table-driven code generation for scalar intrinsics #208

wants to merge 1 commit into from

Conversation

echesakov
Copy link
Contributor

Motivation 1: To support table-driven approach during code generation of "simple" scalar intrinsics (the ones that don't require non-trivial instruction lookup or moving the values between operand registers and destination register).

Motivation 2: To support special (non table-driven) codegen of intrinsics with 3 operands (e.g. BitwiseSelect).

When I implemented these logics for scalar intrinsics and 3 operands intrinsics I found that functions CodeGen::genHWIntrinsic and CodeGen::genSpecialIntrinsic have at least 50% of the identical code so I moved the shared preparatory logic (that looks up intrinsic ID and category, the corresponding operand nodes, and base type) to a separate class HWIntrinsic.

This way I could remove special codegen for NI_ArmBase_LeadingZeroCount NI_ArmBase_Arm64_LeadingSignCount NI_ArmBase_Arm64_LeadingZeroCount.

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Nov 22, 2019
@echesakov
Copy link
Contributor Author

@CarolEidt @tannergooding PTAL

cc @dotnet/jit-contrib

… between table-driven and special intrinsics in jit/hwintrinsiccodegenarm64.cpp
Copy link
Member

@erozenfeld erozenfeld left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@sandreenko sandreenko left a comment

Choose a reason for hiding this comment

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

LGTM, a few nits.

{
assert(op2 == nullptr);

GenTreeArgList* list = op1->AsArgList();
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible that somebody uses GenTreeArgList to encode 2 args form?

GT_LIST
+--* arg1
\--* GT_LIST
    +--* arg2
    \--* nullptr

Copy link
Contributor

Choose a reason for hiding this comment

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

This should not happen. The list form is supposed to be used only for more than 2 args. And hopefully not for long...

Copy link
Contributor

Choose a reason for hiding this comment

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

Welcome to the new repo @mikedn 😃

@mikedn
Copy link
Contributor

mikedn commented Nov 27, 2019

Kind of late to the party... The removal of GT_LIST I'm working on might make some of this a bit redundant. The plan is for SIMD and HWINTRINSIC nodes to store the number of operands and offer operand access by index. For example (names TBD):

node->GetNumUses();
node->GetOp(1);

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

@echesakov
Copy link
Contributor Author

I have to delete/re-create my fork of the runtime repo - I will re-open this PR after that.

@echesakov echesakov closed this Dec 2, 2019
@ghost ghost locked as resolved and limited conversation to collaborators Dec 11, 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants