Skip to content

[LSRA][RyuJIT] Add support to allow either of op1 or op2 to be considered as reg optional rather than Lower fixing one of the operands as reg optional #6358

Closed

Description

@sivarv

Right now reg-optional operand support requires Lower marking a specific operand as reg-optional operand. Lower cannot know apriori which operand is not going to get a register and can only make a guess using heuristics. Analysis indicates, there are quite a few cases which are missed as a result.

Rather than fixing a particular operand as reg-optional it would be advantageous if Lower just specified the max number of operands that could be considered as reg-optional on the parent node and marking all operands that are permissible to be reg-optional.

Example1:
consider GT_ADD(op1, op2).
GT_ADD:
srcRegOptionalCount = 1
Mark both op1 and op2 as reg-optional

Example2:
Consider GT_MUL(op1, containable immed)
Code generated is: imul TargetReg, [mem], immed

Here op1 can be reg optional.
GT_MUL:
srcRegOptionalCount = 1
mark op1 as reg optional.

LSRA needs to ensure while allocating no more than one operand of GT_ADD is treated as reg-optional and hence will be considered as contained memory operand.

Some initial thoughts on how to implement this:
Each ref position now also carries with it a ptr to GenTree where it is consumed.
LSRA while creating def position for GT_ADD will update use positions of op1 and op2 to refer to GT_ADD.
During reg allocation, LSRA will decrement srcRegOptionalCount whenever any of its reg optional operands at use is not allocated a reg. LSRA will not consider a use position as reg option if srcRegOptionalCount of its parent node becomes zero.

It is possible that a reg is allocated to both op1 and op2 since free registers were available. Later one of op1 or op2 could be selected for spilling. allocateBusyReg() also needs to be modified to make sure we don't end up having both the operands in memory - particularly for tree temps because tracked lcls are always marked spill after.

category:cq
theme:register-allocator
skill-level:expert
cost:medium

Activity

  1. self-assigned this
    on Jul 20, 2016
  2. changed the title [-][LSRA][RyuJIT] Add support to allow either of op1 or op2 to be considered as reg optional without Lower specifying it[/-] [+][LSRA][RyuJIT] Add support to allow either of op1 or op2 to be considered as reg optional rather than Lower fixing one of the operands as reg optional[/+] on Jul 20, 2016
  3. transferred this issue fromdotnet/coreclron Jan 31, 2020
  4. added this to the Future milestone on Jan 31, 2020
  5. ghost added
    backlog-cleanup-candidateAn inactive issue that has been marked for automated closure.
    on Feb 10, 2023
  6. ghost closed this as completedon Feb 24, 2023
  7. ghost locked as resolved and limited conversation to collaborators on Mar 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area-CodeGen-coreclrCLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMIbacklog-cleanup-candidateAn inactive issue that has been marked for automated closure.enhancementProduct code improvement that does NOT require public API changes/additionsno-recent-activityoptimizationtenet-performancePerformance related issue

    Type

    No type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions