Hi, all Can someone help review this patch.
1. Bug descript The hash function for VN_BINARY_EXPR will first canonicalize the expression. as the following code in file opt_vn_expr_taxonomy.h:509 size_t hash() { Is_True(!has_bottom_opnd() && !has_top_opnd(), ("Cannot hash() with Top() or Bottom() operand!")); _canonicalize(); return _opc + (_vn[0].ordinal() << 4) + (_vn[1].ordinal() << 8); } Following is the canonicalize code in opt_vn_expr.cxx:754 void VN_BINARY_EXPR::_canonicalize() { // Puts operands in increasing order of VN if commutative, // and change opcode for inequalities such that we always // use LT or LE and never GT or GE. // OPCODE opc1 = OPCODE_commutative_op(_opc); if (opc1 != OPCODE_UNKNOWN && _vn[0] > _vn[1]) { Switch_Vn_Opnd(_vn[0], _vn[1]); _opc = opc1; } else { const OPERATOR opr = OPCODE_operator(_opc); if (opr == OPR_GE) { Switch_Vn_Opnd(_vn[0], _vn[1]); _opc = OPCODE_make_op(OPR_LE, OPCODE_rtype(_opc), OPCODE_desc(_opc)); } else if (opr == OPR_GT) { Switch_Vn_Opnd(_vn[0], _vn[1]); _opc = OPCODE_make_op(OPR_LT, OPCODE_rtype(_opc), OPCODE_desc(_opc)); } } } // VN_BINARY_EXPR::_canonicalize These code would like to do one of the 2 things as in the comments a. put operands in increasing order of VN if commutative b. use LT or LE and never GT or GE For the expression v0 < v1 where Value_number(v0) = 3 , Value_number(v2)= 1 . we try to get it’s hash number. as it satisfy opc1 != OPCODE_UNKNOWN && _vn [0] > _vn[1], it will canonicalize to v1 > v0 The second time we try to get it’s hash number, in this time , this expression satisfy rule a, it will jump to rule b, use LT or LE . So v1 > v0 will canonicalize to v0 < v1 again. We got a different hash number from the first time. The hash number is not unique for this expression. Actually, the relative operators are not consider to be commutatived. Only binary operations that the operands can be exchange and the operator keep the same are commutatived operations, just like plus, multiple, etc. Do not apply rule a for relative operator(LT, LE, GT, GE) should be a solution. Index: osprey/be/opt/opt_vn_expr.cxx =================================================================== --- osprey/be/opt/opt_vn_expr.cxx (revision 3901) +++ osprey/be/opt/opt_vn_expr.cxx (working copy) @@ -759,24 +759,15 @@ // use LT or LE and never GT or GE. // OPCODE opc1 = OPCODE_commutative_op(_opc); - if (opc1 != OPCODE_UNKNOWN && _vn[0] > _vn[1]) + const OPERATOR opr = OPCODE_operator(_opc); + if (opc1 == _opc && _vn[0] > _vn[1]) { Switch_Vn_Opnd(_vn[0], _vn[1]); - _opc = opc1; } - else + else if (opr == OPR_GE || opr == OPR_GT ) { - const OPERATOR opr = OPCODE_operator(_opc); - if (opr == OPR_GE) - { - Switch_Vn_Opnd(_vn[0], _vn[1]); - _opc = OPCODE_make_op(OPR_LE, OPCODE_rtype(_opc), OPCODE_desc(_opc)); - } - else if (opr == OPR_GT) - { - Switch_Vn_Opnd(_vn[0], _vn[1]); - _opc = OPCODE_make_op(OPR_LT, OPCODE_rtype(_opc), OPCODE_desc(_opc)); - } + Switch_Vn_Opnd(_vn[0], _vn[1]); + _opc = opc1; } } // VN_BINARY_EXPR::_canonicalize -- yongchong ------------------------------------------------------------------------------ Better than sec? Nothing is better than sec when it comes to monitoring Big Data applications. Try Boundary one-second resolution app monitoring today. Free. http://p.sf.net/sfu/Boundary-dev2dev _______________________________________________ Open64-devel mailing list Open64-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/open64-devel