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
