Skip to content

[TODO-List-Cleanup] Delete obsolete code from gtSetMultiOpOrder#70216

Merged
BruceForstall merged 1 commit intodotnet:mainfrom
SingleAccretion:HWI-Costing
Jun 7, 2022
Merged

[TODO-List-Cleanup] Delete obsolete code from gtSetMultiOpOrder#70216
BruceForstall merged 1 commit intodotnet:mainfrom
SingleAccretion:HWI-Costing

Conversation

@SingleAccretion
Copy link
Contributor

@SingleAccretion SingleAccretion commented Jun 3, 2022

Remove references to preserving previous behavior.

Use the generalized Sethi-Ullman algorithm for computing the "level" of multi-operand nodes.

What's the explanation for the algorithm?

The problem being solved is that of how many registers does an N-ary node need in order to be evaluated.

First, we observe the numbers for the simplest case of a binary operator:

op1(n) OP(r) op2(m)

If n > m, we need n registers to evaluate "OP"
If n =< m, we need m + 1 registers, m for "op2" and one register to hold "op1" while "op2" is being evaluated

This is the same as saying r = max(n, m + 1)

The N-ary case can be reduced down to a series of binary ones, for example, for a 4-ary node we would have:

    OP
    /\
   /  \
  /    \
 op1   /\
      /  \
     /    \
   op2    /\
         /  \
        /    \
      op3    op4

Thus, to find the number of registers we would walk this tree in post-order, evaluating each binary node with the formula derived above.

Such a post-order walk is equivalent to the reversed linear loop over the operands implemented in this change.

Closes #6622. Addresses the last TODO-List-Cleanup.

A small number of positive diffs.

@ghost ghost added area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI community-contribution Indicates that the PR has been added by a community member labels Jun 3, 2022
@ghost
Copy link

ghost commented Jun 3, 2022

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

Issue Details

Remove references to preserving previous behavior.

Use the generalized Sethi-Ullman algorithm for computing the "level" of multi-operand nodes.

What's the explanation for the algorithm?

The problem being solved is that of how many registers does an N-ary node need in order to be evaluated.

First, we observe the numbers for the simplest case of a binary operator:

op1(n) OP(r) op2(m)

If n > m, we need n registers to evaluate "OP"
If n =< m, we need m + 1 registers, m for "op2" and one register to hold "op1" while "op2" is being evaluated

This is the same as saying r = max(n, m + 1)

The N-ary case can be reduced down to a series of binary ones, for example, for a 4-ary node we would have:

Operands on the left execute before those on the right

    OP
    /\
   /  \ 
  /    \
 op1   /\
      /  \
     /    \
   op2    /\
         /  \
        /    \
      op3    op4

Thus, to find the number of registers we would walk this tree in post-order, evaluating each binary node with the formula derived above.

Such a post-order walk is equivalent to the reversed linear loop over the operands implemented in this change.

Closes #6622.

Author: SingleAccretion
Assignees: -
Labels:

area-CodeGen-coreclr, community-contribution

Milestone: -

@SingleAccretion SingleAccretion changed the title [TODO-List-Cleanup] Remove obsolete code from gtSetMultiOpOrder [TODO-List-Cleanup] Delete obsolete code from gtSetMultiOpOrder Jun 3, 2022
@SingleAccretion SingleAccretion marked this pull request as ready for review June 3, 2022 21:18
@SingleAccretion
Copy link
Contributor Author

Will assume the Build Linux arm64 Release NativeAOT timeout is not related.

@dotnet/jit-contrib

Remove references to preserving previous behavior.

Use a better algorithm for calculating the "level" of MultiOp nodes.
@BruceForstall BruceForstall merged commit a40b002 into dotnet:main Jun 7, 2022
@SingleAccretion SingleAccretion deleted the HWI-Costing branch June 7, 2022 08:56
@ghost ghost locked as resolved and limited conversation to collaborators Jul 7, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unify cost computation for GT_LIST nodes

2 participants