A few small jit changes. #31816
A few small jit changes. #31816
Conversation
sandreenko
added
the
area-CodeGen-coreclr
| @@ -16858,6 +16858,9 @@ CORINFO_CLASS_HANDLE Compiler::gtGetStructHandleIfPresent(GenTree* tree) | |||
| case GT_OBJ: | |||
| structHnd = tree->AsObj()->GetLayout()->GetClassHandle(); | |||
| break; | |||
| case GT_BLK: | |||
There was a problem hiding this comment.
We use this path only in optIsCSEcandidate, but later optIsCSEcandidate doesn't know how to work with these opcodes, so it doesn't produce any diffs.
There was a problem hiding this comment.
Interesting, normally the layouts used by BLK do not have a class handle.
There was a problem hiding this comment.
I usually get such BLK nodes from OBJ nodes for structs that do not contain GC pointers.
I'll check if I see them in master.
There was a problem hiding this comment.
Ah, I've managed to confuse myself. Yes, BLKs can have class handles now, it was my plan to have handles only in OBJs but it didn't happen.
|
PTAL @dotnet/jit-contrib. |
src/coreclr/src/jit/lsraxarch.cpp
Outdated
| { | ||
| return 0; | ||
| } | ||
| assert(indirTree->TypeGet() != TYP_STRUCT); // Don't expect a struct type here. |
There was a problem hiding this comment.
It would be more clear to say why such an indir is not expected here - // struct typed indirs are expected to be contained or something like that
There was a problem hiding this comment.
Thanks, done, I will change that condition soon, so it will become clearer in the next PRs as well.
src/coreclr/src/jit/lower.cpp
Outdated
| { | ||
| assert(ret->OperIs(GT_RETURN)); | ||
|
|
||
| #if !defined(TARGET_64BIT) | ||
| if (ret->TypeGet() == TYP_LONG) | ||
| { | ||
| GenTree* op1 = ret->gtGetOp1(); | ||
| GenTree* op1 = ret->gtOp1; |
There was a problem hiding this comment.
Seems odd to go back to direct data member access. It was, is and will always be a bad programming practice for common data structures such as IR ones.
There was a problem hiding this comment.
You are right, I forgot that gtGetOp1 is declared for GenTree, not for GenTreeOp). Will revert that.
Thank you.
Thanks, I will wait until your PR is merged and then resolve the conflicts. |
sandreenko
force-pushed
the
aFewSmallChanges
branch
from
d3d78ee
to
f2fcda7
Compare
|
The PR was rebased and updated. |
ghost
locked as resolved and limited conversation to collaborators
Fix SPMI replay with COMPlus_JitDump, add a few new asserts, clean a few expressions.
No diffs x64 (SPC, spmi pri1 tests).