this is my suggestion:
1. find one special char that gas can accept but not part of
C/C++/Fortran (note that it could be lang specific), say this is $
2. use anon$ instead of anon
3. #define this string in config.h or something appropriate
4. replace all hard coded use of "anon"
then your change is the right fix, not continue previous bad practice.
Sun
On Fri, Apr 20, 2012 at 3:05 PM, Gang Yu <yugang...@gmail.com> wrote:
> On Fri, Apr 20, 2012 at 2:38 PM, Sun Chan <sun.c...@gmail.com> wrote:
>>
>> It is one thing to allow FE to generate temps with special substring.
>> It it another to assume that special substring always mean compiler
>> generated temps. They are not logically equivalent.
>> Sun
>
>
> Maybe I do not make things clear in my last post. Now, I will put more
> details.
>
> The source code in wgen_spin_symbol.cxx
>
> 1569ST*
> 1570Create_ST_For_Tree (gs_t decl_node)
> 1571{
> 1572 TY_IDX ty_idx;
> 1573 ST* st = NULL;
> 1574 char *name;
> 1575 char tempname[32];
> 1595 if (gs_tree_code(decl_node) == GS_RESULT_DECL) {
> 1596 sprintf(tempname, ".result_decl_%d", gs_decl_uid(decl_node));
> 1597 name = tempname;
> 1598 }
> 1599 else if ((gs_tree_code(decl_node) == GS_FUNCTION_DECL ||
> 1600 gs_tree_code(decl_node) == GS_PARM_DECL ||
> 1601 (gs_tree_code(decl_node) == GS_VAR_DECL &&
> 1602 gs_decl_asmreg(decl_node) >= 0)) &&
> 1603 gs_decl_name(decl_node) != 0)
> 1604 name = (char *) gs_identifier_pointer (gs_decl_name (decl_node));
> 1605 else if (gs_decl_assembler_name (decl_node) && gs_decl_name
> (decl_node))
> 1606 name = (char *) gs_identifier_pointer (gs_decl_assembler_name
> (decl_node));
> 1607 else if (gs_decl_name (decl_node))
> 1608 name = (char *) gs_identifier_pointer (gs_decl_name (decl_node));
>
> 1609 else {
> 1610 sprintf(tempname, "anon%d", ++anon_count);
> 1611 name = tempname;
> 1612 }
> ...
>
> when control goes here, we could say the compiler generate temp var in two
> cases. line 1595-1598 and line 1609-1612, i.e, the green part. common part
> "name = tempname" can be used as the indicate.
>
> We just only want to set the temp var to SCLASS_AUTO for those generated in
> "else" part, i.e, 1609-1612, these names contains "anon", without other flag
> to indicate, we just use this name prefix. So, we can have the following
> change:
>
> 1777 if (name != tempname || strncmp(tempname,"anon",4)) { //
> Not a temp var, or temp var for result_decl, no change
> 1778 sclass = SCLASS_FSTATIC;
> 1779 eclass = EXPORT_LOCAL;
> 1780 } else { // Yes, these are temp vars we want to change
> 1781 sclass = SCLASS_AUTO;
> 1782 eclass = EXPORT_LOCAL;
> 1783 }
> 1784 }
> 1785 if (name != tempname || strncmp(tempname,"anon",4))
> 1786 level = GLOBAL_SYMTAB;
> 1787 else
> 1788 level = CURRENT_SYMTAB;
>
> Regards
> Gang
>
>>
>> On Fri, Apr 20, 2012 at 2:01 PM, Gang Yu <yugang...@gmail.com> wrote:
>> > Hi, Gautam
>> >
>> > Thanks for the review.
>> >
>> > You post a good questtion. In my patch, you may note
>> >
>> > + if (name != tempname || strncmp(tempname,"anon",4)) {
>> >
>> > "name != tempname" address your care.
>> >
>> > Walk though the wgen_spin_symbol.cxx:1611, you may note that
>> >
>> > 1609 else {
>> > 1610 sprintf(tempname, "anon%d", ++anon_count);
>> > 1611 name = tempname;
>> > 1612 }
>> > name == tempname indicate that this is FE generated temp var rather
>> > than var
>> > in user program.
>> > Regards
>> > Gang
>> >
>> >
>> > On Fri, Apr 20, 2012 at 1:54 PM, Gautam Chakrabarti
>> > <gautam.c...@yahoo.com>
>> > wrote:
>> >>
>> >> Hi,
>> >>
>> >> Thanks for working on this issue. But this change is not clean. The
>> >> change
>> >> is essentially assuming special semantics based on a variable name.
>> >> What if
>> >> a user variable is named anon?
>> >>
>> >> -Gautam
>> >>
>> >> From: Gang Yu <yugang...@gmail.com>
>> >> To: open64-devel <open64-devel@lists.sourceforge.net>
>> >> Sent: Thursday, April 19, 2012 2:01 AM
>> >> Subject: [Open64-devel] review request for fix bug965[WGEN]
>> >>
>> >> Hi,
>> >>
>> >> Could a gatekeeper please help review the fix for
>> >> bug965?(http://bugs.open64.net/show_bug.cgi?id=965)
>> >>
>> >> This is a code-size issue for open64, i.e, the compiler generate
>> >> garbage
>> >> code although it still make it right.
>> >>
>> >> a cut-down case below:
>> >>
>> >> static inline __attribute__((always_inline)) int
>> >> x86_this_cpu_constant_test_bit(unsigned int nr,
>> >> const unsigned long *addr)
>> >> {
>> >> unsigned long *a = (unsigned long *)addr + nr / 64;
>> >>
>> >> return ((1UL << (nr % 64)) & ({ typeof(*a) pfo_ret__; switch
>> >> (sizeof(*a))
>> >> {
>> >> case 1: asm("mov" "b ""%%""gs"":" "%P" "1"",%0" : "=q" (pfo_ret_\
>> >> _) : "m" (*a)); break; case 2: asm("mov" "w ""%%""gs"":" "%P" "1"",%0"
>> >> :
>> >> "=r"
>> >> (pfo_ret__) : "m" (*a)); break; case 4: asm("mov" "l ""%%""gs""\
>> >> :" "%P" "1"",%0" : "=r" (pfo_ret__) : "m" (*a)); break; case 8:
>> >> asm("mov"
>> >> "q
>> >> ""%%""gs"":" "%P" "1"",%0" : "=r" (pfo_ret__) : "m" (*a)); break\
>> >> ; default: __bad_percpu_size(); } pfo_ret__; })) != 0;
>> >> }
>> >> opencc -c anon.c -keep
>> >> yug@jupiter:~/work/codesize> readelf -s anon.o
>> >>
>> >>
>> >> Symbol table '.symtab' contains 8 entries:
>> >> Num: Value Size Type Bind Vis Ndx Name
>> >> 4: 0000000000000000 8 OBJECT LOCAL DEFAULT 3 anon1
>> >> x86_this_cpu_constant_test_bit is non-called inline keyword function,
>> >> so
>> >> there should generate nothing symbols in this object file, however,
>> >> anon1 appears unexpected.
>> >> Analysis:
>> >>
>> >> When we examine the WGEN code, we found that the code snipper like
>> >> these:
>> >>
>> >> 1569ST*
>> >> 1570Create_ST_For_Tree (gs_t decl_node)
>> >> 1571{
>> >> ...
>> >> 1609 else {
>> >> 1610 sprintf(tempname, "anon%d", ++anon_count);
>> >> 1611 name = tempname;
>> >> 1612 }
>> >> ...
>> >> 1750 if (gs_tree_public (decl_node)) {
>> >> ...
>> >> 1776 else {
>> >> 1777 sclass = SCLASS_FSTATIC;
>> >> 1778 eclass = EXPORT_LOCAL;
>> >> 1779 }
>> >> 1780 level = GLOBAL_SYMTAB;
>> >> 1781 }
>> >>
>> >> this is to say, when current gs_tree is not public, we generate such a
>> >> variable as File-Scope(static) global variable.
>> >> However, anon1 appears as the temporary variable in a computation.
>> >>
>> >> U8U8LDID 0 <2,4,pfo_ret__> T<9,.predef_U8,8>
>> >> U8STID 0 <1,51,anon1> T<9,.predef_U8,8> {line: 1/7}
>> >> U8INTCONST 1 (0x1)
>> >> I4I4LDID 0 <2,1,nr> T<4,.predef_I4,4>
>> >> I4INTCONST 63 (0x3f)
>> >> I4BAND
>> >> U8SHL
>> >> U8U8LDID 0 <1,51,anon1> T<9,.predef_U8,8>
>> >> U8BAND
>> >> U8INTCONST 0 (0x0)
>> >> I4U8NE
>> >> I4RETURN_VAL {line: 0/0}
>> >> and we do find the producer for anon
>> >> 1609 else {
>> >> 1610 sprintf(tempname, "anon%d", ++anon_count);
>> >> 1611 name = tempname;
>> >> 1612 }
>> >> we should make it SCLASS_AUTO in LOCAL_SYMTAB.
>> >>
>> >> Suggested patch:
>> >> osprey/wgen/wgen_spin_symbol.cxx -- ff56fe0..b24a800 100644
>> >> --- a/osprey/wgen/wgen_spin_symbol.cxx
>> >> +++ b/osprey/wgen/wgen_spin_symbol.cxx
>> >> @@ -1774,10 +1774,18 @@ Create_ST_For_Tree (gs_t decl_node)
>> >> eclass = EXPORT_PREEMPTIBLE;
>> >> }
>> >> else {
>> >> + if (name != tempname || strncmp(tempname,"anon",4)) {
>> >> sclass = SCLASS_FSTATIC;
>> >> eclass = EXPORT_LOCAL;
>> >> + } else {
>> >> + sclass = SCLASS_AUTO;
>> >> + eclass = EXPORT_LOCAL;
>> >> + }
>> >> }
>> >> - level = GLOBAL_SYMTAB;
>> >> + if (name != tempname || strncmp(tempname,"anon",4))
>> >> + level = GLOBAL_SYMTAB;
>> >> + else
>> >> + level = CURRENT_SYMTAB;
>> >> }
>> >> else {
>> >> #ifdef KEY
>> >>
>> >> As an example, I show the codesize comparison to gcc generation and the
>> >> source code in the attach, we can see this is really bad for code-size.
>> >> Could a gatekeeper please help a review? thanks in advance.
>> >>
>> >>
>> >> Regards
>> >> Gang
>> >>
>> >>
>> >>
>> >> ------------------------------------------------------------------------------
>> >> For Developers, A Lot Can Happen In A Second.
>> >> Boundary is the first to Know...and Tell You.
>> >> Monitor Your Applications in Ultra-Fine Resolution. Try it FREE!
>> >> http://p.sf.net/sfu/Boundary-d2dvs2
>> >> _______________________________________________
>> >> Open64-devel mailing list
>> >> Open64-devel@lists.sourceforge.net
>> >> https://lists.sourceforge.net/lists/listinfo/open64-devel
>> >>
>> >>
>> >
>> >
>> >
>> > ------------------------------------------------------------------------------
>> > For Developers, A Lot Can Happen In A Second.
>> > Boundary is the first to Know...and Tell You.
>> > Monitor Your Applications in Ultra-Fine Resolution. Try it FREE!
>> > http://p.sf.net/sfu/Boundary-d2dvs2
>> > _______________________________________________
>> > Open64-devel mailing list
>> > Open64-devel@lists.sourceforge.net
>> > https://lists.sourceforge.net/lists/listinfo/open64-devel
>> >
>
>
------------------------------------------------------------------------------
For Developers, A Lot Can Happen In A Second.
Boundary is the first to Know...and Tell You.
Monitor Your Applications in Ultra-Fine Resolution. Try it FREE!
http://p.sf.net/sfu/Boundary-d2dvs2
_______________________________________________
Open64-devel mailing list
Open64-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/open64-devel
