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

Reply via email to