Z: remove unused and redundant register from object evaluator #19860
Conversation
|
@r30shah please review when possible. Thanks. |
ehsankianifar
force-pushed
the
Z_RemoveUnusedRegistersFromGenHeapAlloc
branch
from
ee8100c
to
122a837
Compare
| } | ||
|
|
||
| iCursor = generateS390BranchInstruction(cg, TR::InstOpCode::BRC, TR::InstOpCode::COND_BNE, node, callLabel, iCursor); | ||
| iCursor = generateRILInstruction(cg, TR::InstOpCode::CLFI, node, enumReg, (1 << 16), iCursor); |
There was a problem hiding this comment.
To eliminate the need for a register (tmp)
|
Hi @ehsankianifar please update the commit body with appropriate explanation of what the change is about and shorten the commit title withing 72 letters. |
ehsankianifar
force-pushed
the
Z_RemoveUnusedRegistersFromGenHeapAlloc
branch
from
122a837
to
90c1a7a
Compare
|
@ehsankianifar The build you launched was only ran on x86, not LoZ (#19860 (comment)). I am still suspicious of some of the registers you removed as there are many different type of cases this will be used so I am going to do thorough review today. Given the scope of area this change will be touching, please launch internal jenkins build on Linux on Z with JDK11 and JDK21 for sanity and extended test target (functional / system and openjdk). Also launch Java 8 vmfarm build on z/OS and Linux on Z with c,p target as well. |
|
Thanks @r30shah for checking that. I start a new build right now |
ehsankianifar
force-pushed
the
Z_RemoveUnusedRegistersFromGenHeapAlloc
branch
from
90c1a7a
to
e1e6e7d
Compare
There was a problem hiding this comment.
Thanks @ehsankianifar for going in the evaluator and cleaning up some part of the code. Overall looked OK to me, but I would like to give one more look specially around the use of Scratch register manager. I have requested some minor changes (Mostly clarifying comments and renaming some of the variable names). Please make this changes as seperate single commit so it is easier to review.
ehsankianifar
force-pushed
the
Z_RemoveUnusedRegistersFromGenHeapAlloc
branch
2 times, most recently
from
0c60909
to
520ea62
Compare
|
@r30shah I addressed the comments in a separate commit as you requested. I can squash the commits after the review or leave it as it is. The tests are passing for the java 8 with c,p tests target on zos and zLinux (build number 34097). |
|
sanity.functional tests were aborted on both jdk21 and jdk11 builds. both passed on rerun (grinder 42320 and 42321) |
ehsankianifar
force-pushed
the
Z_RemoveUnusedRegistersFromGenHeapAlloc
branch
from
520ea62
to
94f29f7
Compare
|
@ehsankianifar Can you squash the commits ? |
VMnewEvaluator uses more registers than it needs. Some registers like zeroReg are initialized but never used. Some registers line dataSizeReg are redundant. This commit uses existing registers as much as possible and removes all the unused or redundant registers. It also uses scratch register manager in genHeapAlloc for better register allocation. Signed-off-by: Ehsan Kiani Far <ehsan.kianifar@gmail.com>
ehsankianifar
force-pushed
the
Z_RemoveUnusedRegistersFromGenHeapAlloc
branch
from
94f29f7
to
b4719f8
Compare
|
@r30shah commits squashed! |
|
jenkins test sanity zlinux jdk21 |
Repurpose registers when no longer needed.
Remove unused registers.
Use scratch register manager inside genHeapAlloc