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

Reply via email to