Repository navigation
JIT: Support assignments of multi-reg values to enregistered promoted structs #34105
Description
Currently, calls or other operations (such as MUL_LONG on 32-bit platforms) that return multiple registers must be stored (either to the stack or elsewhere). This means that promoted structs (whose fields have been promoted to allow them to be separately tracked, optimized and enregistered) will be forced to the stack.
This is the underlying issue in #5112 and #8571, but support for this would also enable MUL_LONG to be supported without forcing the destination to be in memory, as well as enabling hardware intrinsics such as Bmi2.MultiplyNoFlags to return a ValueTuple and allow the result to go directly to a pair of registers.
category:cq
theme:structs
skill-level:expert
cost:large
Activity
CarolEidt commentedon Mar 26, 2020
@terrajobst - The idea of having an intrinsic return a
ValueTuplewas mentioned in the context ofBmi2.MultiplyNoFlags, but comments here: #11782 (comment) and here: #11782 (comment) indicated that it was not in keeping with API guidelines, yet in a recent API review I believe you indicated that it would be OK. Can you advise?terrajobst commentedon Mar 26, 2020
Tuples: we can use tuples in APIs and as of recently we did in a few cases. The general guidance is: don't use them as parameters and only use them when the number of values if 4 or less and we're sure we never need to add more. For the most part, we prefer custom structs because we can add members.
CarolEidt commentedon Mar 26, 2020
Thanks @terrajobst. I'll proceed on the assumption that tuples would be a reasonable way to address the problem of returning multi-register values in intrinsics.
tannergooding commentedon Mar 26, 2020
A possible downside of tuples would be it takes the good name if we have a better type in the future.
For example,
Multiplyfor x86 isint32 * int32 = int64, on so we don't need a tuple, butMultiplyfor x64 can beint64 * int64 = int128.If we support it via a
(long hi, ulong lo)tuple today, we will have to come up with an alternative name if/when we support a properInt128/UInt128type.I don't think this should be a blocker, but it is something we might want to take into consideration when proposing these APIs.
CarolEidt commentedon Mar 26, 2020
@tannergooding - we already have that issue. I'm experimenting with supporting
mulxwith a tuple result, but we already have a version that takes twoulongs and returns a singleulong. So for the purposes of experimentation I've added aMultiplyNoFlags2. Just curious why the 32-bit version doesn't provide aulongresult - is it to be consistent with the 64-bit version, or was it due to concerns with longs being forced to memory on 32-bit targets?tannergooding commentedon Mar 26, 2020
I think it was to mirror the C/C++ API actually: https://software.intel.com/sites/landingpage/IntrinsicsGuide/#text=mulx
CarolEidt commentedon Mar 27, 2020
The fundamental issue here is that we need to be able to take a result from a single node that produces multiple registers, and assign them to the corresponding fields of a promoted struct. This is supported reasonably well currently when the target is going to be in memory anyway (on stack or otherwise). However when the target is a local struct that otherwise could be in registers, we force it to memory.
Multi-reg localVars
The primary issue is how to represent a definition of a struct localVar that has been promoted, but which is being defined by a node that produces multiple registers. Uses don't have quite the same issues; we're already dealing with these as
GT_FIELD_LISTs.Option 1: (What I'm experimenting with now) is to continue to represent these as they are currently done, as a single lclVar node. This is already handled in liveness analysis - it updates the liveness for the fields when the whole struct is used or defined. Issues:
GT_LCL_VARnode.regNumberSmalls.Option2: Add a new kind of lclVar node - e.g. GT_LCL_VAR_TUPLE that explicitly represents N locals that are assigned or used together.
Option3: Add a parent node that has
GT_STORE_LCL_VARchildren, likeGT_FIELD_LISTGT_ASG. It requires the children of a node to consume values produced by another child of that node.@dotnet/jit-contrib - thoughts?
P.S. There may be a better place for this discussion (suggestions welcome).
sandreenko commentedon Apr 1, 2020
I am experimenting with it as well, but my case is simpler:
ASG(struct LCL_VAR with 1 promoted field, call struct). The master branch is currently lucky if the field type is long (x64), because in this case we do retypingstruct parent LCL_VAR->long parent LCL_FIELD -> long child LCL_VAR, but after my struct changes it won't be enregistered.I was trying to substitute all manipulations with the parent LCL_VAR (that should be defs only, all usages should use fields, otherwise it can't be promoted independently). It was straightforward in valuenum, harder in SSA.
So I prefer option 1 with my current perspective because options 2 and 3 won't solve the case that I see (especially option 2).
Enregister multireg lclVars (#36862)
Enregister multireg lclVars