Atomic-free JNI work #2039
Atomic-free JNI work #2039
Conversation
|
I've examined the 32-bit and 64-bit generated code, and they are correct for both entering and exitting the VM. |
| // ctor for Trg1Src2 + condReg only has the preced form, if you don't pass a preceding instruction the following instruction will end up as the first instruction after the prologue | ||
| generateTrg1Src2Instruction(cg(), TR::InstOpCode::and_r, callNode, tempReg2, tempReg1, tempReg2, cr0Reg, /* FIXME */ cg()->getAppendInstruction()); | ||
| } | ||
| TR_ASSERT(J9_PUBLIC_FLAGS_VM_ACCESS >= LOWER_IMMED && J9_PUBLIC_FLAGS_VM_ACCESS <= UPPER_IMMED, "VM access bit must be immediate"); |
There was a problem hiding this comment.
This assert will not fire unless we are in a debug build. If you want something that runs in production use TR_ASSERT_FATAL.
There was a problem hiding this comment.
Which is preferred here do you think? If the value gets out of range, we'll encode a bogus instruction and likely crash, at which point a debug build would show the problem. On the other hand, these two asserts aren't likely to have any effect on codegen performance.
There was a problem hiding this comment.
I would think we want a TR_ASSERT_FATAL here. I'd rather us fail the compilation right away with a full backtrace than almost surely crash at runtime, or yet worse get undefined behavior. We'll make the same assert on Z.
There was a problem hiding this comment.
Sounds good - I'll make the change (and add it to X where I'm already assuming it, but not asserting it).
Fix releaseVMAccess on PPC to properly use compare instead of and. Assert if the VM access bit doesn't fit into an immediate to prevent a constant change from unknowingly bloating the direct JNI path. Also add the immediate value assertion on AMD64. Signed-off-by: Graham Chapman <graham_chapman@ca.ibm.com>
|
@fjeremic Ready for re-review. If you're comfortable merging this, I'll remove Julian. |
|
Jenkins test sanity xlinux,plinux,aix,win |
|
Windows builds failed due to infrastructure issues on the machine: |
| // FIXME: Apparently I'm the first one to ever use a Trg1Src2 record-form instruction... | ||
| // ctor for Trg1Src2 + condReg only has the preced form, if you don't pass a preceding instruction the following instruction will end up as the first instruction after the prologue | ||
| generateTrg1Src2Instruction(cg(), TR::InstOpCode::and_r, callNode, tempReg2, tempReg1, tempReg2, cr0Reg, /* FIXME */ cg()->getAppendInstruction()); | ||
| } |
There was a problem hiding this comment.
Although not applicable anymore, andis_r instruction can be used for the case where J9_PUBLIC_FLAGS_VM_ACCESS falls in the upper 16-bit.
There was a problem hiding this comment.
On the off chance we ever feel the need to change the bit value, we can re-assess. It didn't seem like a good idea to maintain a bunch of dead code in these functions. I think every platform will be able to represent 0x20 as an immediate today.
Fix releaseVMAccess on PPC to properly use compare instead of and.
Assert if the VM access bit doesn't fit into an immediate to prevent a
constant change from unknowingly bloating the direct JNI path.
Also add the immediate value assertion on AMD64.
Signed-off-by: Graham Chapman graham_chapman@ca.ibm.com