Hi, Gautam
Thanks for the review.
You post a good question. 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
