Introduce local variables with unknown size#122638
Conversation
Adds lvaLclSymbolicSize for handling locals that may have an unknown size at compile time (SIZE_UNKNOWN). Adds various assertions to areas with assumptions based on concrete size values. Updates parts of value numbering and SSA building to use the symbolic size of the type.
github-actions
bot
added
the
area-CodeGen-coreclr
dotnet-policy-service
bot
added
the
community-contribution
|
@dotnet/arm64-contrib |
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
|
@dotnet/arm64-contrib @dotnet/jit-contrib Please could I have a review for this patch? It's a precursor for supporting the rest of the local variable implementation for SVE |
The ValueSize class represents the size of type for optimization purposes. It allows symbolic representations of sizes that don't have an exact numerical value at compile time, but have some fixed value at runtime. This value must be equivalent for all instances of the same type. The patch replaces any instances of exact sizes being used to construct value numbers with processing that supports ValueSize instead. SSA Semantics are applied with the following considerations: * A load/store can be considered entire when the load/store ValueSize is equivalent to the source/destination ValueSize, and the offset of the operation is zero. Creates a complete definition. * A load/store on a structure can be considered a partial load/store when the ValueSize of the load/store is not equivalent to the source/destination local variable's ValueSize and/or the offset of the operation is not zero. Creates a use-assign definition. * Complete definitions of non-exact ValueSizes propagate definitions as usual. * Partial definitions of non-exact ValueSizes create unique value numbers, as the bounds of the load/store operation cannot be determined. The patch also applies ValueSize to TYP_SIMD on ARM64. ValueSize kinds are added for Vector and Mask types which have unknown size at compile time when supported by the ARM64 Scalable Vector Extension.
|
I've pushed some further value numbering implementation based on your suggestions, and some assertions firing in my branch after introducing |
jakobbotsch
left a comment
jakobbotsch
left a comment
There was a problem hiding this comment.
This is looking good to me now, with some nits
* Move ValueSize::FromJitType to gentree.cpp * Demote assertions to debug only
* Remove TARGET_ARM64 ifdefs * Remove assertion * Return NoVN for non-entire vector load/store * Assume inexact field stores are always definitions * Add tests for value numbering Vector<T>
* Handle stores that "may" affect a field, and we cannot tell at compile time * Remove test case
|
@snickolls-arm Is this ready for another review? |
Yes this is ready, thanks. |
|
@SingleAccretion Do you have any other feedback? |
SingleAccretion
left a comment
SingleAccretion
left a comment
There was a problem hiding this comment.
The change LGTM modulo two comments.
* Base expression VN on local type rather than store type * Implement value numbers for bit casting to a variable size
|
Thanks! I think we will need to have a further discussion on the general representation of structs containing vector fields and what we will do about field offsets, sizes and the internal representation of these in the JIT. I think it has potential to become a very infectious change and I am hoping we can find a way to avoid that. |
Yes absolutely. I can open a separate issue for this if that suits and we can continue discussion there? Some thoughts I've had to start, which I'll write up in more detail: I think it's not necessarily required to solve this to get the feature off the ground, at least for JIT and R2R. The EE will always be reporting exact structure offsets, as it reads VL and sets it in the method table of For R2R modes, I found safeguards in the interface that prompt recompilation for methods that involve If we want to generate complete VL agnostic code in future in NativeAOT mode (i.e. we want to add SVE to the AOT baseline), then we'd need to work out the ABI for these sorts of structures in full, and support this in the |
6 participants
Adds
ValueSizeclass for handling sizes that may be unknown at compile time during phases such as value numbering, that aren't concerned with the exact numeric value (only properties such as equivalence etc.). Adds various assertions to areas of value numbering and SSA building with assumptions based on exact size values. AddsLclVarDsc::lvValueSizeto produce aValueSizefor a local variable, which provides the criteria for which locals have unknown size at compile time.