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
>------------------------------------------------------------------------------
Cloud Services Checklist: Pricing and Packaging Optimization
This white paper is intended to serve as a reference, checklist and point of
discussion for anyone considering optimizing the pricing and packaging model
of a cloud services business. Read Now!
http://www.accelacomm.com/jaw/sfnl/114/51491232/
_______________________________________________
Open64-devel mailing list
Open64-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/open64-devel
