Looks good to me. Please go ahead.

2012/3/14 Gang Yu <yugang...@gmail.com>

> Thanks very much for the comment:
>
> On Wed, Mar 7, 2012 at 11:13 AM, Jian-Xin Lai <laij...@gmail.com> wrote:
>
>> Two comments:
>> 1. This patch is not generic enough. If the type tenths is shor or
>> other, it still fails
>>
>
> Yes, when tenths is short, it still fails at O2 or above. the patch does
> not handle the case:
>
>    U8U8LDID 78 <1,9,.preg_U8> T<9,.predef_U8,4> # __udivdi3.return
>   I8CVTL 16
>  I4STID 72 <1,4,.preg_I4> T<4,.predef_I4,2> # tenths {line: 1/27}
> result of
>    U8U8LDID 78 <1,9,.preg_U8> T<9,.predef_U8,4> # __udivdi3.return
> I8CVTL 16
> is still 64bit with two paired TNs, so I4STID 72 is expanded to
> paired, this is not expected.
>
> I get a revised patch on this issue, instead of handle the case at LDID
> site, we handle it at the STID site. we check the returned result and stid
> type to get the right final result. And so, we avoid changing the interface.
>
> Below is the modified patch:
>
> diff --git a/osprey/be/cg/cgexp_internals.h
> b/osprey/be/cg/cgexp_internals.h
> index d707336..1479610 100644
> --- a/osprey/be/cg/cgexp_internals.h
> +++ b/osprey/be/cg/cgexp_internals.h
> @@ -145,6 +145,7 @@ extern void Expand_Shuffle (OPCODE opcode, TN* result,
> TN* src1,
>  TN* Get_TN_Pair( TN* );
>  TN* Create_TN_Pair( TN*, TYPE_ID );
>  void Create_TN_Pair( TN*, TN* );
> +void Delete_TN_Pair( TN* );
>  #endif /* TARG_X8664 */
>  #if defined(TARG_IA64) || defined(TARG_LOONGSON)
> diff --git a/osprey/be/cg/whirl2ops.cxx b/osprey/be/cg/whirl2ops.cxx
> index 38ef367..1c42188 100644
> --- a/osprey/be/cg/whirl2ops.cxx
> +++ b/osprey/be/cg/whirl2ops.cxx
> @@ -2774,6 +2774,19 @@ Handle_STID (WN *stid, OPCODE opcode)
>         TN* result_hi = Get_TN_Pair( result );
>         Expand_Immediate( result_hi, Gen_Literal_TN(0,4), FALSE, &New_OPs
> );
>        }
> +
> +      // open64.net bug952.
> +      // When the kid is 64bit result with paired TN but the stid_type
> +      // is 4byte size or less, We delete the useless higher 32bit tn
> +      // by unpair the result. this will help pass the pair check in the
> +      // later expansion routines.
> +
> +      if (OP_NEED_PAIR(kid0_type) &&
> +          Get_TN_Pair(result) != NULL &&
> +          MTYPE_byte_size(stid_type) < MTYPE_byte_size(kid0_type)) {
> +        Delete_TN_Pair(result);
> +      }
> +
>  #endif // TARG_X8664
>  #endif // IA_32
> diff --git a/osprey/be/cg/x8664/expand.cxx b/osprey/be/cg/x8664/expand.cxx
> index 4c90c48..a22a630 100644
> --- a/osprey/be/cg/x8664/expand.cxx
> +++ b/osprey/be/cg/x8664/expand.cxx
> @@ -282,6 +282,11 @@ TN* Get_TN_Pair( TN* key )
>    return pair;
>  }
> +void Delete_TN_Pair( TN *key)
> +{
> +  Is_True( Get_TN_Pair( key ) != NULL, ("Delete_TN_Pair: higher 32-bit is
> NULL") );
> +  TN_MAP_Set( _TN_Pair_table, key, NULL );
> +}
>  void Create_TN_Pair( TN* key, TN* pair )
>  {
> Would you please kindly help more a review?
>
> Regards
> Gang
>
>



-- 
Regards,
Lai Jian-Xin
------------------------------------------------------------------------------
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