JIT: Refactor physical promotion store decomposition slightly #88182
Conversation
Introduce a LocationAccess helper class to create derived accesses off of the destination and source locations for the store. Unify all the code that looks for regularly promoted fields in this class, and use it consistently for all the derived accesses. Also update terminology from "assignment" to "store" in a few places, and add a "(last use)" string for fields when decomposing block stores.
dotnet-issue-labeler
bot
added
the
area-CodeGen-coreclr
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue DetailsIntroduce a LocationAccess helper class to create derived accesses off of the destination and source locations for the store. Unify all the code that looks for regularly promoted fields in this class, and use it consistently for all the derived accesses. Also update terminology from "assignment" to "store" in a few places, and add a "(last use)" string for fields when decomposing block stores. Precursor to #88109.
|
|
cc @dotnet/jit-contrib PTAL @EgorBo Some very minor diffs. They occur because of the change to no longer keep regular promoted locals in the decomposition plan, and instead handle that when we get to the corresponding field offset (inside STMT00002 ( 0x00F[E-] ... ??? )
[000008] DAC-------- ▌ STORE_LCL_VAR struct<System.DateTimeOffset, 16> V02 loc0
[000006] --C-G------ └──▌ CALL struct System.DateTimeOffset:get_UtcNow():System.DateTimeOffset
Processing block operation [000008] that involves replacements
Fields of [000008] in range [000..016) need to be read back: cannot decompose store
V66 (V02.[000..002)) marked
V67 (V02.[008..016)) marked
STMT00027 ( 0x015[E-] ... ??? )
[000095] DA--------- ▌ STORE_LCL_VAR struct<System.DateTimeOffset, 16>(P) V06 tmp3
▌ short field V06._offsetMinutes (fldOffset=0x0) -> V38 tmp35 (last use)
▌ long field V06._dateTime (fldOffset=0x8) -> V39 tmp36
[000009] ----------- └──▌ LCL_VAR struct<System.DateTimeOffset, 16> V02 loc0
Processing block operation [000095] that involves replacements
Source replacement V66 (V02.[000..002)) is stale. Will read it back before copy.
- V38 (field V06._offsetMinutes (fldOffset=0x0)) <- V66 (V02.[000..002))
+ dst+000 <- V66 (V02.[000..002))
Source replacement V67 (V02.[008..016)) is stale. Will read it back before copy.
- V39 (field V06._dateTime (fldOffset=0x8)) <- V67 (V02.[008..016))
+ dst+008 <- V67 (V02.[008..016))
Block op remainder: [002..008)
=> Remainder strategy: retain a full block op
+ Skipping dst+000 <- V66 (V02.[000..002)); it is up-to-date in its struct local and will be handled as part of the remainder
+ Skipping dst+008 <- V67 (V02.[008..016)); it is up-to-date in its struct local and will be handled as part of the remainder
New statement:
STMT00027 ( 0x015[E-] ... ??? )
- [000345] -A--------- ▌ COMMA void
+ [000339] -A--------- ▌ COMMA void
[000335] DA--------- ├──▌ STORE_LCL_VAR int V66 tmp63
[000334] ----------- │ └──▌ LCL_FLD short V02 loc0 [+0]
- [000344] -A--------- └──▌ COMMA void
+ [000338] -A--------- └──▌ COMMA void
[000337] DA--------- ├──▌ STORE_LCL_VAR long V67 tmp64
[000336] ----------- │ └──▌ LCL_FLD long V02 loc0 [+8]
- [000343] -A--------- └──▌ COMMA void
- [000095] DA--------- ├──▌ STORE_LCL_VAR struct<System.DateTimeOffset, 16>(P) V06 tmp3
- ├──▌ short field V06._offsetMinutes (fldOffset=0x0) -> V38 tmp35 (last use)
- ├──▌ long field V06._dateTime (fldOffset=0x8) -> V39 tmp36
- [000009] ----------- │ └──▌ LCL_VAR struct<System.DateTimeOffset, 16> V02 loc0
- [000342] -A--------- └──▌ COMMA void
- [000339] DA--------- ├──▌ STORE_LCL_VAR short V38 tmp35
- [000338] ----------- │ └──▌ LCL_VAR short V66 tmp63
- [000341] DA--------- └──▌ STORE_LCL_VAR long V39 tmp36
- [000340] ----------- └──▌ LCL_VAR long V67 tmp64
+ [000095] DA--------- └──▌ STORE_LCL_VAR struct<System.DateTimeOffset, 16>(P) V06 tmp3
+ ▌ short field V06._offsetMinutes (fldOffset=0x0) -> V38 tmp35 (last use)
+ ▌ long field V06._dateTime (fldOffset=0x8) -> V39 tmp36
+ [000009] ----------- └──▌ LCL_VAR struct<System.DateTimeOffset, 16> V02 loc0 (On a side note: the EE is telling us here that padding of |
|
Realized there was a bit more to clean up so pushed another commit. |
| { | ||
| if (genTypeSize(type) == 1) | ||
| { | ||
| return m_indirFlags & ~GTF_IND_UNALIGNED; |
There was a problem hiding this comment.
Maybe it's better to be done inside gtNewIndir itself?
There was a problem hiding this comment.
Not sure, creating these derived indirections probably doesn't happen in any other place than decomposition and block morph. And even if it does we probably don't even need to remove this flag... I'm just being overly cautious here.
I wouldn't call it prohibited, but seems nice to be consistent. I'm still writing "assignment decomposition" half the time I refer to this part of physical promotion |
…#88182) Introduce a LocationAccess helper class to create derived accesses off of the destination and source locations for the store. Unify all the code that looks for regularly promoted fields in this class, and use it consistently for all the derived accesses. Also update terminology from "assignment" to "store" in a few places, and add a "(last use)" string for fields when decomposing block stores.
…dotnet#88182)" This reverts commit 4501f38.
ghost
locked as resolved and limited conversation to collaborators
Introduce a LocationAccess helper class to create derived accesses off of the destination and source locations for the store. Unify all the code that looks for regularly promoted fields in this class, and use it consistently for all the derived accesses.
Also update terminology from "assignment" to "store" in a few places, and add a "(last use)" string for fields when decomposing block inits.
Precursor to #88109.