The current code uses ST_IDX_level(ST_st_idx(st)) == CURRENT_SYMTAB to
determine whether an auto variable is uplevel ref. My fix follows the old
way to do that. What is the right way? Thanks.

On Mon, Dec 20, 2010 at 7:46 PM, Sun Chan <sun.c...@gmail.com> wrote:

> I thought there is a bit in the symbol table that tells whether the
> symbol is uplevel ref. Your method seemed to be a hack
> Sun
>
> On Mon, Dec 20, 2010 at 7:25 PM, Jiangzhou HE <hejiangz...@gmail.com>
> wrote:
> > Hi,
> >
> > Can a gatekeeper help to review the patch to bug 702
> > (https://bugs.open64.net/show_bug.cgi?id=702)
> >
> > This is the test case:
> >
> > #include <stdio.h>
> >
> > void foo(int x) {
> >
> >
> >   void far() {
> >     x++;
> >
> >   }
> >
> >   void bar() {
> >
> >     int x2 = x;
> >     far();
> >
> >     printf("%d %d\n", x, x2);
> >   }
> >
> >
> >   bar();
> > }
> >
> >
> > int main() {
> >   foo(1);
> >
> >   return 0;
> > }
> >
> > This is a bug of alias classifying. Because of this bug, in bar(), the
> > variable x is treated as a local variable in current PU, so x gets an own
> > class ID. Therefore, in the call site for far(), no CHI node is created
> for
> > x. Then, by copy propagation, the last argument of printf() is replaced
> by
> > x. Then we can see the wrong output.
> >
> > This is the buggy code:
> >
> >   // If this base_id is a local variable or parameter, it gets its
> >   // own class.
> >   if ((storage_class == SCLASS_AUTO &&
> >        ST_IDX_level(ST_st_idx(st)) == CURRENT_SYMTAB) ||
> >       storage_class == SCLASS_FORMAL                  ||
> >       storage_class == SCLASS_FORMAL_REF              ||
> >       storage_class == SCLASS_REG) {
> >     // Set up the LDA and LDID classes for this variable...
> >
> > It checks whether the symbol is in current level for auto variables, but
> > does not check for formals. I fix the bug in this way:
> >
> > diff --git a/osprey/be/opt/opt_alias_class.cxx
> > b/osprey/be/opt/opt_alias_class.cxx
> > index d4ebc6f..42db9b2 100644
> > --- a/osprey/be/opt/opt_alias_class.cxx
> > +++ b/osprey/be/opt/opt_alias_class.cxx
> > @@ -409,10 +409,10 @@ ALIAS_CLASSIFICATION::New_base_id(const ST *st,
> TY_IDX
> > ty)
> >
> >    // If this base_id is a local variable or parameter, it gets its
> >    // own class.
> > -  if ((storage_class == SCLASS_AUTO &&
> > +  if (((storage_class == SCLASS_AUTO ||
> > +        storage_class == SCLASS_FORMAL ||
> > +        storage_class == SCLASS_FORMAL_REF) &&
> >         ST_IDX_level(ST_st_idx(st)) == CURRENT_SYMTAB) ||
> > -      storage_class == SCLASS_FORMAL                  ||
> > -      storage_class == SCLASS_FORMAL_REF              ||
> >        storage_class == SCLASS_REG) {
> >      // Set up the LDA and LDID classes for this variable...
> >      ALIAS_CLASS_MEMBER *ldid_item = New_alias_class_member(id);
> > @@ -490,7 +490,8 @@ ALIAS_CLASSIFICATION::New_base_id(const ST *st,
> TY_IDX
> > ty)
> >            // uplevel reference and therefore must be treated as
> >            // global because its address can be stored into a global,
> >            // and it may contain the address of a global. Non-uplevel
> > -          // references with SCLASS_AUTO are handled as locals above.
> > +          // references with SCLASS_AUTO, SCLASS_FORMAL or
> > +           // SCLASS_FORMAL_REF are handled as locals above.
> >            // Including uplevel references here is the fix for 555533.
> >            // 634200 was a milder case of the same problem, and only
> >            // part of the fix was required to get that one right; that
> > @@ -498,6 +499,8 @@ ALIAS_CLASSIFICATION::New_base_id(const ST *st,
> TY_IDX
> > ty)
> >            // parameters; 555533 makes clear that we have to go
> >            // all the way and treat them as globals.
> >            storage_class == SCLASS_AUTO    ||
> > +           storage_class == SCLASS_FORMAL  ||
> > +           storage_class == SCLASS_FORMAL_REF  ||
> >            // storage class can be UNKNOWN for constant data because CG
> may
> >            // already have run for an earlier PU and in the process
> lowered
> >            // string (and maybe other) constants to .rodata.
> Unfortunately,
> >
> > Thanks!
> >
> > --
> > Jiangzhou HE (何江舟)
> >
> > Institute of High-Performance Computing
> > Department of Computer Science and Technology
> > Tsinghua University, Beijing, China
> >
> >
> >
> ------------------------------------------------------------------------------
> > Lotusphere 2011
> > Register now for Lotusphere 2011 and learn how
> > to connect the dots, take your collaborative environment
> > to the next level, and enter the era of Social Business.
> > http://p.sf.net/sfu/lotusphere-d2d
> > _______________________________________________
> > Open64-devel mailing list
> > Open64-devel@lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/open64-devel
> >
> >
>



-- 
Jiangzhou HE (何江舟)

Institute of High-Performance Computing
Department of Computer Science and Technology
Tsinghua University, Beijing, China
------------------------------------------------------------------------------
Lotusphere 2011
Register now for Lotusphere 2011 and learn how
to connect the dots, take your collaborative environment
to the next level, and enter the era of Social Business.
http://p.sf.net/sfu/lotusphere-d2d
_______________________________________________
Open64-devel mailing list
Open64-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/open64-devel

Reply via email to