The change looks fine, please go ahead.
Regards,
Gautam
________________________________
From: Gang Yu <yugang...@gmail.com>
To: Gautam Chakrabarti <gautam.c...@yahoo.com>
Cc: open64-devel <open64-devel@lists.sourceforge.net>
Sent: Sunday, December 11, 2011 4:40 AM
Subject: Re: Review request for fix bug929[GCC-SPIN]
Thanks, Gautam
The suggested way works.
--- a/osprey-gcc-4.2.0/gcc/stor-layout.c
+++ b/osprey-gcc-4.2.0/gcc/stor-layout.c
@@ -1850,8 +1850,16 @@ layout_type (tree type)
finish_record_layout (rli, /*free_p=*/true);
#ifdef KEY
- if (flag_spin_file && gspin_invoked(type))
+ if (flag_spin_file && gspin_invoked(type)) {
+ /* bug929 open64.net. Don't miss the type align field update
+ after the layout is re-calulated. */
+ gs_t align_node;
+ align_node = __gs (IB_INT);
+ _gs_n (align_node, TYPE_ALIGN (type));
+ gs_set_operand ((gs_t) GS_NODE (type), GS_TYPE_ALIGN, align_node);
+
gs_x (type);
+ }
#endif
}
Actually, it does the same thing but in different places. Your suggested way
plays in good shape, thanks.
Regards
Gang
On Sat, Dec 10, 2011 at 1:13 AM, Gautam Chakrabarti <gautam.c...@yahoo.com>
wrote:
Hi Gang,
>
>If you look at rest of the code in this function gs_x_1() in tree.c, you will
>see that we are using similar logic for setting many other flags, i.e. set it
>only if it is null. The idea is that it may not always be correct to update
>the flag the way the change is trying to do below.
>
>It looks like you already know where GCC updates the flag, you say in
>stor-layout.c. Then typically the GS_NODE is updated right at that point.
>Whenever you see the align getting updated, you can check if the tree has
>already been converted to spin (using gspin_invoked()), and using
>gs_set_operand to update the align. For similar instances, you can search for
>gs_set_flag_value in the source code, and you would see the usage.
>
>Please see if the above works.
>
>Gautam
>
>
>
>From: Gang Yu <yugang...@gmail.com>
>To: open64-devel <open64-devel@lists.sourceforge.net>; Gautam Chakrabarti
><gautam.c...@yahoo.com>
>Sent: Thursday, December 8, 2011 5:55 PM
>Subject: Re: Review request for fix bug929[GCC-SPIN]
>
>
>
>Hi, Gautam:
>
> Since you are the gcc expert and the gatekeeper for FE. Would you please
>kindly review this patch? thank you very much.
>
>Regards
>Gang
>
>
>
>On Sat, Dec 3, 2011 at 3:52 PM, Gang Yu <yugang...@gmail.com> wrote:
>
>Hi,
>>
>> Could a gatekeeper please help review the fix for
>>bug929(http://bugs.open64.net/show_bug.cgi?id=929)?
>>
>>A cut-down bug case:
>>
>>typedef long __kernel_time_t;
>>typedef long __kernel_suseconds_t;
>>typedef signed long long s64;
>>typedef __kernel_suseconds_t suseconds_t;
>>struct timespec;
>>extern struct timespec ns_to_timespec(const s64 nsec);
>>struct other {
>> struct timespec *s;
>> int x;
>>} o;
>>void foo(void) {
>> o.x++;
>>}
>>struct timeval {
>> __kernel_time_t tv_sec;
>> __kernel_suseconds_t tv_usec;
>>};
>>struct timespec {
>> __kernel_time_t tv_sec;
>> long tv_nsec;
>>};
>>
>>struct timeval ns_to_timeval(const s64 nsec)
>>{
>> struct timespec ts = ns_to_timespec(nsec);
>> struct timeval tv;
>> tv.tv_sec = ts.tv_sec;
>> tv.tv_usec = (suseconds_t) ts.tv_nsec / 1000;
>> return tv;
>>}
>>
>>opencc build with CG assertion:
>>
>>### Assertion failure at line 1678 of
>>/fc/proj/ctires/open64/o64guru/src/Fri/trunk/osprey/be/cg/lra.cxx:
>>### Compiler Error in file bug929-cutdown.c during Local Register Allocation
>>phase:
>>### LRA: Error in Add_Avail_Reg
>>opencc INTERNAL ERROR:
>>/fc/proj/ctires/open64/o64guru/OPEN64_X86_DBG/LATEST/bits//lib/gcc-lib/x86_64-open64-linux/5.0/be
>> returned non-zero status 1
>>
>>Analysis:
>>
>>Although this bug is asserted at CG phase, in fact, it is already wrong at
>>SPIN generation. SPIN DECL snippet for struct timespec is as follows:
>>
>>@gs_tree_type GS_RECORD_TYPE<2467408> { (0)GS_TCC<2467520> GS_TCC_TYPE
>>(3)IB_BIT_VECTOR<2467524> GS_TYPE_LANG_FLAG_0 (5)IB_STRING<2467876> "VOID"
>>(9)IB_INT<2467884> 8 (10)IB_INT<2467892> -1 (12)IB_INT<2467900> 0
>>(22)IB_BIT_VECTOR<2467908>
>> @gs_type_name GS_IDENTIFIER_NODE<2467808> { (0)GS_TCC<2467848>
>>GS_TCC_EXCEPTIONAL (3)IB_BIT_VECTOR<2467852> (4)IB_STRING<2467868>
>>"timespec"
>>
>>
>>this is unexpected, since timespec has two fields, tv_sec and tv_nsec, at
>>x86_64 , both are 64bit alignment and timespec has no packed or useralgin
>>propertie. So, the struct should be 64bit align.
>>
>>Investigation shows, gs_type_align property is updated at function
>>gs_x_1, tree.c
>>10794
>>10795 /* align */
>>10796 if (gs_operand((gs_t) GS_NODE(t), GS_TYPE_ALIGN) == NULL) {
>>10797 gs_t align_node;
>>10798 align_node = __gs (IB_INT);
>>10799 _gs_n (align_node, TYPE_ALIGN (t));
>>10800 gs_set_operand ((gs_t) GS_NODE (t), GS_TYPE_ALIGN, align_node);
>>10801 }
>>
>>This logic says, when we have not updated the ALIGN field, update it from gcc
>>tree. The right logic should be, we always update the ALIGN propertie if it
>>is not updated or we get a different value from gcc tree. So, the suggested
>>patch:
>>
>>osprey-gcc-4.2.0/gcc/tree.c -- 791fd43..87e478b 100644
>>--- a/osprey-gcc-4.2.0/gcc/tree.c
>>+++ b/osprey-gcc-4.2.0/gcc/tree.c
>>@@ -10793,7 +10793,10 @@ gs_x_1 (tree t, HOST_WIDE_INT seq_num)
>> gs_set_operand((gs_t) GS_NODE(t), GS_TYPE_USER_ALIGN, gs_x_1(NULL,
>>seq_num));
>> /* align */
>>- if (gs_operand((gs_t) GS_NODE(t), GS_TYPE_ALIGN) == NULL) {
>>+ if (gs_operand((gs_t) GS_NODE(t), GS_TYPE_ALIGN) == NULL ||
>>+ // bug929 open64.net. Don't miss the type align field update
>>+ // after the layout is recalulated by gcc stor-layout.
>>+ (TYPE_ALIGN (t) != (unsigned int) gs_n(gs_operand((gs_t)
>>GS_NODE(t), GS_TYPE_ALIGN)))) {
>> gs_t align_node;
>> align_node = __gs (IB_INT);
>> _gs_n (align_node, TYPE_ALIGN (t));
>>
>>Would a gatekeeper please help a review? thanks
>>
>>
>>Regards
>>Gang
>>
>
>
>------------------------------------------------------------------------------
Learn Windows Azure Live! Tuesday, Dec 13, 2011
Microsoft is holding a special Learn Windows Azure training event for
developers. It will provide a great way to learn Windows Azure and what it
provides. You can attend the event by watching it streamed LIVE online.
Learn more at http://p.sf.net/sfu/ms-windowsazure
_______________________________________________
Open64-devel mailing list
Open64-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/open64-devel
