sorry for missing this. This change is not target independent is it?
Sun
On Tue, Jun 28, 2011 at 11:08 AM, Jian-Xin Lai <laij...@gmail.com> wrote:
> This patch looks fine to me. Please go ahead.
>
> 2011/6/25 Xiaohua Zhang <xiaohua_zh...@yahoo.com>:
>> Hi,
>>
>> Could a gatekeeper help to review the fix for bug 799?
>>
>> Thanks,
>> -Xiaohua
>>
>> The small test case is:
>>
>> #include <stdio.h>
>> int rval = 0;
>> typedef struct S { double i; } S;
>> S f(void);
>> typedef struct {
>> int f1;
>> struct S f2;
>> } S1;
>> S1 m;
>> struct S t = { 0.0 };
>> int main(void)
>> {
>> t.i = 12.345;
>> m.f2 = f(); // return value should be in xmm0
>> if (m.f2.i != t.i) {
>> rval = -1;
>> printf("Failed!\n");
>> }
>> return rval;
>> }
>> struct S f(void)
>> {
>> return t;
>> }
>>
>> The WHIRL tree for "m.f2 = f()" looks like:
>>
>> MCALL 126 <1,53,f> # flags 0x7e {line: 1/20}
>> MMLDID -1 <1,49,.preg_return_val> T<53,S,8>
>> MSTID 8 <1,54,m> T<55,.anonymous.1,8> <field_id:2> {line: 0/0}
>>
>> After the RETURN_VAL & MLDID/MSTID lowering:
>>
>> MCALL 126 <1,53,f> # flags 0x7e {line: 1/20}
>> I8I8LDID 1 <1,5,.preg_I8> T<5,.predef_I8,8> # $r1
>> I8STID 8 <1,54,m> T<5,.predef_I8,8> {line: 0/0}
>> F8F8LDID 17 <1,11,.preg_F8> T<11,.predef_F8,8> # $f0
>> F8STID 16 <1,54,m> T<11,.predef_F8,8> {line: 0/0}
>>
>> The return value (S) is returned by two register: $r1 and $f0.
>> By the ABI, the f()'s return value (type struct S) should be returned by one
>> floating register $f0.
>> The problem is in lower_return_mstid(), which takes MSTID's type (S1) as
>> return type, didn't consider the field (f2) it accessed. So it used the
>> wrong type (S1)to get return info, and the return value for S2 is by
>> register $r1 and $f0.
>>
>> The fix is to get the accessed field type as return type.
>>
>>
>>
>>
>>
>> ----------------------
>>
>> Index: be/com/wn_lower.cxx
>> ===================================================================
>> --- be/com/wn_lower.cxx (revision 3659)
>> +++ be/com/wn_lower.cxx (working copy)
>> @@ -7239,7 +7239,12 @@
>> ST *preg_st;
>> WN *n_rhs;
>> WN *wn = NULL; // init to prevent upward-exposed use
>> - RETURN_INFO return_info = Get_Return_Info(WN_ty(tree),
>> Complex_Not_Simulated
>> + TY_IDX ty_idx = WN_ty(tree);
>> +
>> + if (WN_field_id (tree) != 0)
>> + ty_idx = get_field_type (ty_idx, WN_field_id (tree));
>> +
>> + RETURN_INFO return_info = Get_Return_Info(ty_idx, Complex_Not_Simulated
>> #ifdef TARG_X8664
>> , last_call_ff2c_abi
>> #endif
>> @@ -7260,7 +7265,7 @@
>> #endif
>> WN *awn = WN_CreateLda(OPR_LDA, Pointer_Mtype, MTYPE_V,
>> WN_store_offset(tree),
>> - Make_Pointer_Type(WN_ty(tree)), WN_st_idx(tree));
>> + Make_Pointer_Type(ty_idx), WN_st_idx(tree));
>> awn = lower_expr(block, awn, actions);
>> WN *n_call = add_fake_parm(call, awn, WN_ty(awn));
>> WN_DELETE_FromBlock(block, call);
>> @@ -7277,9 +7282,9 @@
>> mtype = RETURN_INFO_mtype(return_info, i);
>> desc = mtype;
>> #ifdef KEY
>> - if (MTYPE_byte_size(mtype) > TY_size(WN_ty(tree)) &&
>> !MTYPE_float(mtype)){
>> + if (MTYPE_byte_size(mtype) > TY_size(ty_idx) && !MTYPE_float(mtype)){
>> desc = Mtype_AlignmentClass(1, MTYPE_type_class(mtype));
>> - while (MTYPE_byte_size(desc) < TY_size(WN_ty(tree)))
>> + while (MTYPE_byte_size(desc) < TY_size(ty_idx))
>> desc = Mtype_next_alignment(desc);
>> }
>> #endif
>> @@ -7287,7 +7292,7 @@
>>
>> #if defined(TARG_PPC32)
>> if (preg_st == Int_Preg) {
>> - UINT rem = TY_size(WN_ty(tree)) - i * 4;
>> + UINT rem = TY_size(ty_idx) - i * 4;
>> if (1 == rem) desc = MTYPE_I1;
>> if (2 == rem) desc = MTYPE_I2;
>> }
>>
>>
>>
>> ------------------------------------------------------------------------------
>> All the data continuously generated in your IT infrastructure contains a
>> definitive record of customers, application performance, security
>> threats, fraudulent activity and more. Splunk takes this data and makes
>> sense of it. Business sense. IT sense. Common sense..
>> http://p.sf.net/sfu/splunk-d2d-c1
>> _______________________________________________
>> Open64-devel mailing list
>> Open64-devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/open64-devel
>>
>>
>
>
>
> --
> Regards,
> Lai Jian-Xin
>
> ------------------------------------------------------------------------------
> All of the data generated in your IT infrastructure is seriously valuable.
> Why? It contains a definitive record of application performance, security
> threats, fraudulent activity, and more. Splunk takes this data and makes
> sense of it. IT sense. And common sense.
> http://p.sf.net/sfu/splunk-d2d-c2
> _______________________________________________
> Open64-devel mailing list
> Open64-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/open64-devel
>
------------------------------------------------------------------------------
All of the data generated in your IT infrastructure is seriously valuable.
Why? It contains a definitive record of application performance, security
threats, fraudulent activity, and more. Splunk takes this data and makes
sense of it. IT sense. And common sense.
http://p.sf.net/sfu/splunk-d2d-c2
_______________________________________________
Open64-devel mailing list
Open64-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/open64-devel
