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
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
