Cleanup unrolled block op codegen#27146
Conversation
|
cc @sandreenko |
| storeIns = INS_strh; | ||
| break; | ||
| default: | ||
| storeIns = INS_str; |
There was a problem hiding this comment.
Nit: could you please update this and the next switch with case 4: ... default: unreached() as discussed in https://github.com/dotnet/coreclr/pull/27035/files#r332302814?
There was a problem hiding this comment.
Sorry, missed that comment. I also forgot to bring some PR feedback commits from there. One moment...
| case 1: | ||
| loadIns = INS_ldrb; | ||
| storeIns = INS_strb; | ||
| #ifdef _TARGET_ARM64_ |
There was a problem hiding this comment.
FWIW this is un unfortunate inconsistency between ARM32 and ARM64. The ARM64 emitter gets it wrong because the instruction/register size is really 4 or 8, never 1 or 2. I fixed something like a while ago for cast instructions but I didn't realize that load/store instructions suffer from the same issue.
CarolEidt
left a comment
CarolEidt
left a comment
There was a problem hiding this comment.
LGTM - thanks for your patience and flexibility in getting this in!
3 participants
Extracted from #27035