KEMBAR78
JIT: Model CPU flag dependencies explicitly in LIR by jakobbotsch · Pull Request #82355 · dotnet/runtime · GitHub
Skip to content

JIT: Model CPU flag dependencies explicitly in LIR #82355

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 9 commits into from
Closed

JIT: Model CPU flag dependencies explicitly in LIR #82355

wants to merge 9 commits into from

Conversation

@jakobbotsch
Copy link
Member

@jakobbotsch jakobbotsch commented Feb 18, 2023 •
edited
Loading

An experiment to see what it would look like. Based on top of #82235.

  • Move GTF_SET_FLAGS into LIR flags as LIR::Flags::ProducesFlags.
  • Nodes in LIR are now allowed to produce flags in addition to values when LIR::Flags::ProducesFlags is set. A node that produces flags can appear in a node that consumes flags, like GT_JCC, GT_SELECTCC, GT_ADD_HI, etc.. If it produces a value it can also appear separately as a normal operand (e.g. GT_ADD_LO usually appears as both a flags def and value def on x86)
  • Flags dependencies are not allowed to overlap. This is now checked by CheckLIR in addition to checking for some flags interference.
  • Teach LIR functions to handle these new dependencies
  • Teach liveness to DCE flag producing nodes that are unused (I don't expect many improvements from this. Maybe a few on x86.)

@ghost ghost assigned jakobbotsch Feb 18, 2023
@jakobbotsch jakobbotsch mentioned this pull request Feb 20, 2023
Merged
@jakobbotsch
Copy link
Member Author

jakobbotsch commented Mar 3, 2023 •
edited
Loading

This was an interesting experiment, but I think overall the complexity is not worth what we get. For most cases it seems to be ok to have the invariant that the flags def comes as the previous node in linear order.
There are some cases where that is not the case today, e.g. some parts of decomposition, and here:

runtime/src/coreclr/jit/lowerarmarch.cpp

Lines 886 to 909 in 86c04cf

// {expr} % {cns}
// Logically turns into:
// let a = {expr}
// if a > 0 then (a & ({cns} - 1)) else -(-a & ({cns} - 1))
// which then turns into:
// and reg1, reg0, #({cns} - 1)
// negs reg0, reg0
// and reg0, reg0, #({cns} - 1)
// csneg reg0, reg1, reg0, mi
GenTree* const neg = comp->gtNewOperNode(GT_NEG, type, dividend2);
neg->gtFlags |= GTF_SET_FLAGS;
BlockRange().InsertAfter(trueExpr, neg);
GenTreeIntCon* cns2 = comp->gtNewIconNode(divisorCnsValueMinusOne, type);
BlockRange().InsertAfter(neg, cns2);
GenTree* const falseExpr = comp->gtNewOperNode(GT_AND, type, neg, cns2);
BlockRange().InsertAfter(cns2, falseExpr);
LowerNode(falseExpr);
mod->ChangeOper(GT_CSNEG_MI);
mod->gtOp1 = trueExpr;
mod->gtOp2 = falseExpr;

But so far we haven't run into problems with these, and we don't need to be able to find the flags def for these cases (yet).

@jakobbotsch jakobbotsch closed this Mar 3, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Apr 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

Copilot code review Copilot Awaiting requested review from Copilot Copilot will automatically review once the pull request is marked ready for review

Assignees

@jakobbotsch jakobbotsch

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@jakobbotsch