search

djrieger / mjplusplus

A compiler for the MiniJava language
http://djrieger.github.io/mjplusplus/doc/doxygen/html/
6 stars 1 forks source link

Fix new_Minus FirmNodeHandler::handle() #84

Closed djrieger closed 9 years ago

djrieger commented 9 years ago

FirmNodeHandler::handle() contains calls to new_Minus. These will probably fail and need to be replaced with new_r_Minus and a reference to the current graph.

maxvogel commented 9 years ago

new_Minus fails because of the assertion: mj++: build/gen/ir/ir/gen_irnode.c:1559: new_d_Minus: Assertion irg_is_constrained_(current_ir_graph, IR_GRAPH_CONSTRAINT_CONSTRUCTION)' failed. However, new_r_Minus wants the current block (instead of a graph) and also fails, when trying to get the current block with the same assertion: mj++: ir/ir/ircons.c:443: get_r_cur_block: Assertion irg_is_constrained_(irg, IR_GRAPH_CONSTRAINT_CONSTRUCTION)' failed.

So this probably won't work. Is it possible for Const nodes to have a negative value? If so, are there any reasons against replacing Minus nodes with Const nodes?

BigPeet commented 9 years ago

We actually do this already in if(is_Minus(node))...

So I see no reason against producing a const there as well. Am 12.12.2014 15:27 schrieb "maxvogel" notifications@github.com:

new_Minus fails because of the assertion: mj++: build/gen/ir/ir/gen_irnode.c:1559: new_d_Minus: Assertion irg_isconstrained(current_ir_graph, IR_GRAPH_CONSTRAINT_CONSTRUCTION)' failed.

However, new_r_Minus wants the current block (instead of a graph) and also fails, when trying to get the current block with the same assertion: mj++: ir/ir/ircons.c:443: get_r_cur_block: Assertion irg_isconstrained(irg, IR_GRAPH_CONSTRAINT_CONSTRUCTION)' failed.

So this probably won't work. Is it possible for Const nodes to have a negative value? If so, are there any reasons against replacing Minus nodes with Const nodes?

— Reply to this email directly or view it on GitHub https://github.com/djrieger/mjplusplus/issues/84#issuecomment-66778655.

maxvogel commented 9 years ago

I just realised, that we cannot produce a const there, because if it was const, both nodes would have been const and therefor handled in a different case. So, new_(r_)Minus seems to be necessary. I'll write to Manuel and Sebastian regarding the firm assertion fail.