Question on arithmetic overflow detection support in C2
Krystal Mok
rednaxelafx at gmail.com
Wed Jun 20 17:28:41 PDT 2012
On Thu, Jun 21, 2012 at 8:03 AM, Vladimir Kozlov <vladimir.kozlov at oracle.com
> wrote:
> Krystal Mok wrote:
>
>> Thanks for the reply! Comments inline below:
>>
>> On 2012-6-21, at 6:13, Vladimir Kozlov <vladimir.kozlov at oracle.com>
>> wrote:
>>
>> Look on output before assert message. It should have problematic ideal
>>> subgraph dump which it can't match.
>>>
>>> The output is:
>>
>
> You did not send output.
>
> Oops, I hit the send button too early ...
Here's the output:
o30 If === o5 o46 o45 [[o31 o32 6 ]] P=0.000001, C=-1.000000
--N: o30 If === o5 o46 o45 [[o31 o32 6 ]] P=0.000001, C=-1.000000
--N: o46 Bool === _ o45 [[o30 ]] [??]
CMPOP 0 CMPOP
CMPOPU 0 CMPOPU
--N: o45 Proj === o43 [[o46 o30 ]] #1
RREGI 0 RREGI
RAX_REGI 0 RAX_REGI
RBX_REGI 0 RBX_REGI
RCX_REGI 0 RCX_REGI
RDX_REGI 0 RDX_REGI
RDI_REGI 0 RDI_REGI
NO_RCX_REGI 0 NO_RCX_REGI
NO_RAX_RDX_REGI 0 NO_RAX_RDX_REGI
STACKSLOTI 100 storeSSI
>
>
>> I think the problem is that you have Proj node in between If and
>>> CheckedAddI nodes so that it can't match 'If' node since it looks for
>>> inputs which produce flags (cr). May be CheckedAddINode::match() is not
>>> enough.
>>>
>>> You're right, that's exactly what's happening.
>>
>
One more question on this one: how does the matcher know that the data
projection should be in the same location as the dst input? There's nowhere
to specify it.
>
>> Also you can't use EMPTY mask for data result projection. Bind it to an
>>> register as DivMod node.
>>>
>>>
> Mask should reflect in what registers could be result. You don't need to
> bind to specific register but you need to return non-empty mask, for
> example, INT_REG_mask(). And for flag projection you need INT_FLAGS_mask().
Ahh, so the register class declaration for RFLAGS moved to x86.ad ... no
wonder I couldn't find it when I searched x86_64.ad.
I'm still getting the same crash after I changed CheckedAddINode::match()
to this:
Node* CheckedAddINode::match(const ProjNode* proj, const Matcher* match) {
uint ideal_reg = proj->ideal_reg();
RegMask rm = RegMask::Empty;
if (proj->_con == sum_proj_num) {
rm = match->checkedAddI_sum_proj_mask();
} else {
assert(proj->_con == overflow_proj_num, "must be sum or overflow
projection");
ideal_reg = Op_RegFlags;
rm = match->overflow_proj_mask();
}
return new (match->C, 1) MachProjNode(this, proj->_con, rm, ideal_reg);
}
Where both RegMasks specified as suggested above. The output log is still
the same.
Thanks,
Kris
>
>
> Vladimir
>
>
> But in the DivMod case, both data projections had to be in fixed
>> registers because that's required by idiv, where as here binding the sum
>> projection to a fixed register (e.g. rax) would be less optimal, right?
>>
>> If only there's a way to express the (Set dest ...) semantic for
>> CheckedAddI's data projection ...
>>
>> Anyway, binding it fixed should at least get the example running. I'll
>> try this first and look for alternative solutions later.
>>
>> And use masm instructions encoding
>>>
>>> ins_encode %{
>>> __ addl($dst$$Register, $src$$Register);
>>> %}
>>>
>>> instead of
>>>
>>> ins_encode(REX_reg_reg(dst, src), OpcP, reg_reg(dst, src));
>>>
>>> Will fix this one. I just copied the code from the basic instruct for
>> AddI, which is still using the old ins_encode form.
>>
>> Thanks,
>> Kris
>>
>> Vladimir
>>>
>>> Krystal Mok wrote:
>>>
>>>> Thank you for all your input, John and Vladimir!
>>>> I'm close to getting some prototype that could actually run...but not
>>>> there just yet.
>>>> I've got Match.addExact(int, int) compiled down to this graph:
>>>> http://dl.iteye.com/upload/**picture/pic/114582/cc7535db-**
>>>> 20d8-3e5d-9abd-b96456c7166c.**png<http://dl.iteye.com/upload/picture/pic/114582/cc7535db-20d8-3e5d-9abd-b96456c7166c.png>
>>>> with the CheckedAddI node resembling the way DivModI works.
>>>> But apparently I'm not getting the Matcher part right yet. I'm getting
>>>> this error message:
>>>> # Internal Error (/home/sajia/temp/hotspot-**comp/src/share/vm/opto/**matcher.cpp:1564),
>>>> pid=16856, tid=1117243712
>>>> # assert(false) failed: bad AD file
>>>> from this stack trace:
>>>> V [libjvm.so+0xb10bea] VMError::report(outputStream*)**+0xfa4
>>>> V [libjvm.so+0xb11f77] VMError::report_and_die()+**0x649
>>>> V [libjvm.so+0x59e706] report_vm_error(char const*, int, char const*,
>>>> char const*)+0x9c
>>>> V [libjvm.so+0x8d4af0] Matcher::Label_Root(Node const*, State*,
>>>> Node*, Node const*)+0x48c
>>>> V [libjvm.so+0x8d4d0f] Matcher::match_tree(Node const*)+0x205
>>>> V [libjvm.so+0x8d6397] Matcher::xform(Node*, int)+0x16d
>>>> V [libjvm.so+0x8d9ea2] Matcher::match()+0xb0e
>>>> V [libjvm.so+0x52e20f] Compile::Code_Gen()+0x91
>>>> V [libjvm.so+0x537678] Compile::Compile(ciEnv*, C2Compiler*,
>>>> ciMethod*, int, bool, bool)+0x101e
>>>> V [libjvm.so+0x47aa8c] C2Compiler::compile_method(**ciEnv*,
>>>> ciMethod*, int)+0xea
>>>> V [libjvm.so+0x53e552] CompileBroker::invoke_**compiler_on_method(**
>>>> CompileTask*)+0x332
>>>> V [libjvm.so+0x540f10] CompileBroker::compiler_**thread_loop()+0x2fa
>>>> V [libjvm.so+0xabed88] compiler_thread_entry(**JavaThread*,
>>>> Thread*)+0x54
>>>> V [libjvm.so+0xac288c] JavaThread::thread_main_inner(**)+0x102
>>>> V [libjvm.so+0xac4f0b] JavaThread::run()+0xef
>>>> V [libjvm.so+0x979557] java_start(Thread*)+0x16f
>>>> The current status of the patch is avaiable here:
>>>> https://gist.github.com/**05f6f33cb01c7f5aedf3<https://gist.github.com/05f6f33cb01c7f5aedf3>
>>>> I haven't done a full-fledged intrinsic for Math.addExact(int, int)
>>>> yet; just as a prototype, I'm faking the use of CmpAddI node right now. At
>>>> the end of Optimize(), I pattern match for the overflow checking logic,
>>>> replace it with a new fake CmpAddI node, then make the actual CheckedAddI
>>>> node from the CmpAddI, finally replace the old AddI and Bool nodes.
>>>> Most of the changes besides the new nodes are for adding the
>>>> overflow/no-overflow conditions to BoolTest and cmpOp.
>>>> If there were separate AddI and CmpAddI nodes, they would have had a
>>>> (Set dst ...) and (Set cr ...) in their match rule. But I can't see how I
>>>> can express it here.
>>>> Regards,
>>>> Kris
>>>>
>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20120621/b40782b1/attachment-0001.html
More information about the hotspot-compiler-dev
mailing list