Question on arithmetic overflow detection support in C2

[Vladimir Kozlov]

Look on output before assert message. It should have problematic ideal subgraph 
dump which it can't match.

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.

Also you can't use EMPTY mask for data result projection. Bind it to an register 
as DivMod node.

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));

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
> 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
> 
> 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