Looks good to me.

2012/7/25 Xiaojing Zhang <xiaojing70...@gmail.com>:
> Hi,
>
>    Could a gatekeeper please help review the fix for bug1000?
> (https://bugs.open64.net/show_bug.cgi?id=1000)
>
> This is an regression caused by rev3973.
>
> 1.  Test case
>
> This bug causes runtime failure on SPEC2006 403.gcc testcase, the minimized
> case list below:
>
> #include <stdio.h>
>
> extern int floor_log2_wide (unsigned long long);
>
> extern int target_flags;
>
> extern FILE *asm_out_file;
>
> int foo (int for_eh) {
>
>   if ((floor_log2_wide ((unsigned long long) (for_eh ? (((target_flags &
> 0x02000000) ? 64 : 32) / 8) : (((target_flags & 0x02000000) ? 64 : 32) /
> 8))))!=0)
>
>    fprintf ((asm_out_file), "\t.align %d\n", 1<<(floor_log2_wide ((unsigned
> long long) 8)));
>
>   }
>
> 2.  IR dump
>
> The whirl IR:
>
>    I4I4NE
>
>    U4INTCONST 8 (0x8)
>
>    U4INTCONST 4 (0x4)
>
>   U4SELECT
>
> U4STID 56 <1,8,.preg_U4> T<8,.predef_U4,4> # <preg> {line: 1/7}
>
>    I4I4LDID 0 <2,1,for_eh> T<4,.predef_I4,4>
>
>    U4INTCONST 0 (0x0)
>
>   I4I4NE
>
>   U8U4LDID 56 <1,8,.preg_U4> T<8,.predef_U4,4> # <preg>
>
> U8STID 57 <1,9,.preg_U8> T<9,.predef_U8,4> # <preg> {line: 1/7}
>
>
> The CGIR is:
>
> [   7, 0] TN150 :- ldc32 (0x8) ;
>
> [   7, 0] TN152 :- ldc32 (0x4) ;
>
> [   7, 0] TN154 :- ld32_n32 (sym:target_flags+0) ; WN id:11 target_flags+0x0
>
> [   7, 0] TN155 :- andi32 TN154 (0x2000000) ;
>
> [   7, 0] TN149 :- mov32 TN152 ; copy
>
> [   7, 0] TN33(%rflags) :- test32 TN155 TN155 ;
>
> [   7, 0] TN149 :- cmovne TN150 TN33(%rflags) ; cond_def
>
> [   7, 0] TN157 :- mov32 TN149 ; copy
>
> [   7, 0] :- store32 TN157 TN4(%rsp) (0x0) ; WN id:13
>
> [   7, 0] :- store32 TN158 TN4(%rsp) (0x4) ; WN id:13
>
> [   7, 0] :- call (sym:floor_log2_wide+0) ; WN
>
> TN158 is the pair of TN157, which is created when handling STID, but has no
> assignment.
>
> 3.   Analysis
>
> In CG expansion phase, when handling LDID, if the wn_class of LDID is preg,
> whether the PAIR need or not is up to the ST_mtype of U8U4LDID.
>
> In whirl2ops.cxx:
>
> 2029 #elif  defined(TARG_X8664)
>
> 2030       if( OP_NEED_PAIR( ST_mtype(WN_st(ldid) ) ) ){
>
> 2031         Expand_Copy( result, ldid_result, ST_mtype(WN_st(ldid) ),
> &New_OPs );
>
> 2032       } else {
>
> 2033         Exp_COPY (result, ldid_result, &New_OPs);
>
> 2034       }
>
> 2035 #else  // TARG_X8664
>
> For U8U4LDID, the ST_mtype is U4, so Exp_COPY is just involved to copy TN149
> to TN157, but for TN158 (the Pair of TN157), nothing to do.
>
> So the OP_NEED_PAIR here should be decided by WN_rtype of ldid, not
> ST_mtype.
>
> 4.   Patch
>
> Index: osprey/be/cg/whirl2ops.cxx
>
> ===================================================================
>
> --- osprey/be/cg/whirl2ops.cxx  (revision 3973)
>
> +++ osprey/be/cg/whirl2ops.cxx  (working copy)
>
> @@ -2027,8 +2027,8 @@
>
>          }
>
>        }
>
> #elif  defined(TARG_X8664)
>
> -      if( OP_NEED_PAIR( ST_mtype(WN_st(ldid) ) ) ){
>
> -        Expand_Copy( result, ldid_result, ST_mtype(WN_st(ldid)), &New_OPs
> );
>
> +      if( OP_NEED_PAIR( WN_rtype(ldid) ) ){
>
> +        Expand_Copy( result, ldid_result,  WN_rtype(ldid), &New_OPs );
>
>        } else {
>
>          Exp_COPY (result, ldid_result, &New_OPs);
>
>        }
>
>
>
> 5.   Explanation
>
> In this patch, for U8U4LDID, the Expand_Copy will call Expand_Split_UOP in
> expand.cxx, and the higher 32-bit of TN is treated as 0.
>
> The CGIR after the patch is:
>
> [   7, 0] TN150 :- ldc32 (0x8) ;
>
> [   7, 0] TN152 :- ldc32 (0x4) ;
>
> [   7, 0] TN154 :- ld32_n32 (sym:target_flags+0) ; WN id:7 target_flags+0x0
>
> [   7, 0] TN155 :- andi32 TN154 (0x2000000) ;
>
> [   7, 0] TN149 :- mov32 TN152 ; copy
>
> [   7, 0] TN33(%rflags) :- test32 TN155 TN155 ;
>
> [   7, 0] TN149 :- cmovne TN150 TN33(%rflags) ; cond_def
>
> [   7, 0] TN160 :- ldc32 (0x0) ;
>
> [   7, 0] TN157 :- mov32 TN149 ; copy
>
> [   7, 0] TN158 :- mov32 TN160 ; copy
>
> [   7, 0] :- store32 TN157 TN4(%rsp) (0x0) ; WN id:10
>
> [   7, 0] :- store32 TN158 TN4(%rsp) (0x4) ; WN id:10
>
> [   7, 0] :- call (sym:floor_log2_wide+0) ; WN
>
>
> Best regards,
>
> Xiao-Jing
>
>
>
> ------------------------------------------------------------------------------
> Live Security Virtual Conference
> Exclusive live event will cover all the ways today's security and
> threat landscape has changed and how IT managers can respond. Discussions
> will include endpoint security, mobile security and the latest in malware
> threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
> _______________________________________________
> Open64-devel mailing list
> Open64-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/open64-devel
>



-- 
Regards,
Lai Jian-Xin

------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
Open64-devel mailing list
Open64-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/open64-devel

Reply via email to