Hi,

Could a gate keeper review the patch for the CVT bug #885 in WOPT?
Thank you very much.

The test case is very simple:
long long foo(int* p) {
  int x = *p;
  return (unsigned int)x;
}

The IR generated by wgen is:
   U8U8LDID 0 <2,1,p> T<53,anon_ptr.,8>
  I4I4ILOAD 0 T<4,.predef_I4,4> T<53,anon_ptr.,8>
 I4STID 0 <2,2,x> T<4,.predef_I4,4> {line: 1/2}
  U8U4LDID 0 <2,2,x> T<8,.predef_U4,4>
 U8RETURN_VAL {line: 1/3}

In WOPT, the initial CODEREP is:
>  LDID U8 U8 sym1v3 0 ty=3503  <u=1 cr7> flags:0x0 b=-1 #p
> I4I4ILOAD 0 ty=402  <u=0 cr8> flags:0x0 b=-1 mu<7/cr5>
>OPR_STID I4 I4 sym3v3 0 <u=1 cr9> b=-1 flags:0x2 pj2 Sid-1       // x = 
>(uint32)*p
chi <> 0x0x80e36f8
>LINE 3___
>  LDID I4 I4 sym3v3 0 ty=402  <u=1 cr9> flags:0x0 b=-1 #x
> U8U4CVT <u=2 cr10> isop_flags:0xc0000 flags:0x0 b=-1
>OPR_STID U8 U8 sym5v3 1 <u=0 cr11> b=-1 flags:0x2 pj2 Sid-1  // $1 = 
>(uint64)(uint32)x

If we enable the propagation on CSE expressions, the coderep will be:
(if we don't turn on prop_cse, the propagation won't happen and the
bug is not exposed)
>   LDID U8 U8 sym1v3 0 ty=3503  <u=1 cr7> flags:0x0 b=-1 #p
>  I4I4ILOAD 0 ty=402  <u=2 cr8> flags:0x0 b=-1 mu<7/cr5>
> U8U4CVT <u=1 cr11> isop_flags:0xc0000 flags:0x0 b=-1
>OPR_STID U8 U8 sym5v3 1 <u=0 cr12> b=-1 flags:0x2 pj2 Sid-1    // $1 = 
>(uint64)(uint32)(*p)

In BDCE phase, the U8U4CVT will be deleted because both U8U4CVT and
I4I4ILOAD are treaded as zero-ext in BITWISE_DCE::Redundant_cvtl():
    if (sign_xtd == opnd->Is_sign_extd())
      return from_bit >= MTYPE_size_min(opnd->Dsctyp());

The IR generated by WOPT is:
   U8U8LDID 49 <1,9,.preg_U8> T<53,anon_ptr.,8> # p
  I8I4ILOAD 0 T<4,.predef_I4,4> T<53,anon_ptr.,8>
 U8STID 1 <1,9,.preg_U8> T<9,.predef_U8,8> # $r1 {line: 1/3}

The zero-ext is converted into sign-ext and finally the movslq is generated.

Here is a change made in opt_bdce.cxx:
Index: osprey/be/opt/opt_bdce.cxx
===================================================================
--- osprey/be/opt/opt_bdce.cxx  (revision 3747)
+++ osprey/be/opt/opt_bdce.cxx  (working copy)
@@ -1122,6 +1122,8 @@
     if (Split_64_Bit_Int_Ops && to_bit == 64)
       return FALSE;
 #ifdef TARG_X8664
+    if (from_bit == 32 && to_bit == 64 && sign_xtd !=
MTYPE_is_signed(opnd->Dsctyp()))
+      return FALSE;
     if (MTYPE_size_min(opnd->Dsctyp()) < 32 && to_bit == 64)
       return !sign_xtd && !opnd->Is_sign_extd() &&
             from_bit >= MTYPE_size_min(opnd->Dsctyp());
@@ -1139,6 +1141,8 @@
     if (Split_64_Bit_Int_Ops && to_bit == 64)
       return FALSE;
 #ifdef TARG_X8664
+    if (from_bit == 32 && to_bit == 64 && sign_xtd !=
MTYPE_is_signed(opnd->Dsctyp()))
+      return FALSE;
     if (MTYPE_size_min(opnd->Dsctyp()) < 32 && to_bit == 64)
       return !sign_xtd && !opnd->Is_sign_extd() &&
             from_bit >= MTYPE_size_min(opnd->Dsctyp());

When the the ext type of CVT doesn't match the ext type of the
ILOAD/LDID, the CVT is not redundant.

After this change, the U8U4CVT is also removed by the final ML WHIRL
emitter because in cvt_rule defined in osprey/be/com/opt_cvtl_rule.cxx
is nop.
So there is another change made to the opt_cvtl_rule.cxx:
Index: osprey/be/com/opt_cvtl_rule.cxx
===================================================================
--- osprey/be/com/opt_cvtl_rule.cxx     (revision 3747)
+++ osprey/be/com/opt_cvtl_rule.cxx     (working copy)
@@ -147,7 +147,7 @@
 INT Need_type_conversion(TYPE_ID from_ty, TYPE_ID to_ty, OPCODE *opc)
 {
 #ifdef TARG_X8664 // bug 2879
-  if (Is_Target_32bit() && from_ty == MTYPE_U4 &&
+  if (from_ty == MTYPE_U4 &&
       MTYPE_is_integral(to_ty) && MTYPE_byte_size(to_ty) == 8) {
     if (opc != NULL)
       *opc = MTYPE_signed(to_ty) ? OPC_I8I4CVT : OPC_U8U4CVT;

With the two changes, the IR generated by wopt is:
   U8U8LDID 49 <1,9,.preg_U8> T<53,anon_ptr.,8> # p
  U8U4ILOAD 0 T<4,.predef_I4,4> T<53,anon_ptr.,8>
 U8STID 1 <1,9,.preg_U8> T<9,.predef_U8,8> # $r1 {line: 1/3}

The final asm code is:
        movl 0(%rdi),%eax

The whole patch is attached. I don't plan to check in this patch into
this release but I'd like to get it reviewed. Thank you very much.

--
Regards,
Lai Jian-Xin

Attachment: 885.patch
Description: Binary data

------------------------------------------------------------------------------
The demand for IT networking professionals continues to grow, and the
demand for specialized networking skills is growing even more rapidly.
Take a complimentary Learning@Cisco Self-Assessment and learn 
about Cisco certifications, training, and career opportunities. 
http://p.sf.net/sfu/cisco-dev2dev
_______________________________________________
Open64-devel mailing list
Open64-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/open64-devel

Reply via email to