Repository navigation
update the location of assert for REG_ZR check - #112294
kunalspathak merged 2 commits into
Conversation
Sorry, something went wrong.
|
@dotnet/jit-contrib @amanasifkhalid |
Sorry, something went wrong.
amanasifkhalid
left a comment
There was a problem hiding this comment.
LGTM, thanks!
Sorry, something went wrong.
| return; | ||
| } | ||
|
|
||
| // reg2 can be alread encoded to zero |
There was a problem hiding this comment.
| // reg2 can be alread encoded to zero | |
| // reg2 can be already encoded to zero |
Sorry, something went wrong.
There was a problem hiding this comment.
ok to fix in next batch? don't want to run CI again.
Sorry, something went wrong.
There was a problem hiding this comment.
Sure
Sorry, something went wrong.
Shouldn't the code that produces the add/sub be doing encodingZRtoSP instead when encoding it? I think it would be better to keep the requirement that reg2 is never ZR, since it's not a supported possible instruction. |
Sorry, something went wrong.
Yeah, the emitIns_R_R_I() is a very common method and was not sure if that's the right thing to do. But this is happening in scenario where we populate base address for predicate register, so let me point fix those 2 places only. |
Sorry, something went wrong.
amanasifkhalid
left a comment
There was a problem hiding this comment.
Agreed that fixing the caller seems better
Sorry, something went wrong.
* main: (41 commits) Automated bump of chrome version (dotnet#112309) Add `GetDeclaringType` to `PropertyDefinition` and `EventDefinition`. (dotnet#111646) Update the System.ComponentModel.Annotations solution to build in VS (dotnet#112313) JIT: initial support for stack allocating arrays of GC type (dotnet#112250) [main] Update dependencies from dotnet/roslyn (dotnet#112260) Update Xcode casing (dotnet#112307) update the location of assert for REG_ZR check (dotnet#112294) Enable `SA1206`: Keyword ordering (dotnet#112303) Address feedback on dense FrozenDictionary optimization (dotnet#112298) Start regular pri-1 tests runs with native AOT (dotnet#111391) Observe exceptions from _connectionCloseTcs (dotnet#112190) Test failure - SendAsync_RequestVersion20_ResponseVersion20 (dotnet#112232) Fix init race in mono_class_try_get_[shortname]_class. (dotnet#112282) Remove repeated call to DllMain (dotnet#112285) Replace bitvector.h/cpp with ptrArgTP type in gc_unwind_x86.h/inl (dotnet#112268) JIT: Limit 3-opt to 1000 swaps per run (dotnet#112259) [main] Update dependencies from dotnet/icu, dotnet/runtime-assets (dotnet#112120) Update dependencies from https://github.com/dotnet/emsdk build 20250205.3 (dotnet#112223) Fix EventPipe on Android CoreClr. (dotnet#112270) Fix exception handling in the prestub worker (dotnet#111937) ...
This probably got exposed with #111451 because that makes more usages of SP than FP. While loading or storing from stack with an offset, we already encode SP -> ZR, but if we have to produce the offset using reserved register, we perform add or sub to populate it. The caller already encodes REG_SP as REG_ZR, but in add/sub, we were expecting the reg2 to be SP or GPR, but later we will encode it to ZR anyway. So, I have just moved the assert to take into account of that.
Fixes: #112278