Deprecate ShrinkWrapping - Part I #3166
Conversation
ShrinkWrapping is currently disabled on all platforms due to functional issues. It has been disabled for several years and it's benefit is questionable at best given the compile time costs. This commit folds away some code related to ShrinkWrapping so as to not cause build breaks when the optimization is removed from OMR. There will be a subsequent "Part II" PR which will remove all the rest of the code which is dependent on OMR. Signed-off-by: Filip Jeremic <[email protected]>
fjeremic
force-pushed
the
remove-shrink-wrapping
branch
from
a657139 to
864dbaf
Compare
|
Preemptively launching builds here: Jenkins test sanity all JDK8 |
| if (!p || p->get((uint32_t)regIndex)) | ||
| cursor = generateMemSrc1Instruction(cg(),TR::InstOpCode::Op_st, firstNode, new (trHeapMemory()) TR::MemoryReference(stackPtr, argSize, TR::Compiler->om.sizeofReferenceAddress(), cg()), machine->getPPCRealRegister(regIndex), cursor); | ||
| cursor = generateMemSrc1Instruction(cg(),TR::InstOpCode::Op_st, firstNode, new (trHeapMemory()) TR::MemoryReference(stackPtr, argSize, TR::Compiler->om.sizeofReferenceAddress(), cg()), machine->getPPCRealRegister(regIndex), cursor); | ||
|
|
There was a problem hiding this comment.
This comment is not related to the changes in this PR but more an observation I'll make about the code that's already there and want to document it somewhere.
The use of om.sizeofReferenceAddress() in this register preserving code seems wrong to me. This code should be preserving the full register size for the target bitness rather than asking the object model what the size of a reference is. As luck (design?) would have it, on 64-bit this always returns '8' and on 32-bit returns '4' regardless of whether compressed refs is enabled or not.
There are some curiosities that need to be sorted out here (like what should sizeOfReferenceAddress be returning from the object model, what breaks if that changes). I'll open a separate issue for that investigation.
There was a problem hiding this comment.
@gita-omr FYI the above comment.
There was a problem hiding this comment.
Thanks. Will wait for the issue that @0xdaryl is going to open.
ShrinkWrapping is currently disabled on all platforms due to functional
issues. It has been disabled for several years and it's benefit is
questionable at best given the compile time costs.
This commit folds away some code related to ShrinkWrapping so as to not
cause build breaks when the optimization is removed from OMR. There
will be a subsequent "Part II" PR which will remove all the rest of the
code which is dependent on OMR.
Issue: eclipse-omr/omr#2107
Signed-off-by: Filip Jeremic [email protected]