from the numbering, it seemed to be sgi bug numbers. Which means Robert Kennedy (the original author of this code) added the comment, for obvious reasons. The code must have a good reason to be there. Please read through the code again and make sure we are looking at uplevel ref the right way Sun
On Mon, Dec 20, 2010 at 8:21 PM, Tianwei <tianwei.sh...@gmail.com> wrote: > > > 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, > > Hi, Jiangzhou, > which class does the "x" in bar belong to? SCLASS_AUTO or > SCLASS_FORMAL? I thought the "x" is SCLASS_FORMAL, but not in current > symbol table level, right? So your statement "x is treated as a local > variable in current PU" is not right. > as the following comments indicate, > // storage class AUTO here signifies that the item is an > // 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. > // 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 > // bug made me mistakenly treat uplevel references as > // parameters; 555533 makes clear that we have to go > // all the way and treat them as globals. > storage_class == SCLASS_AUTO || > I think existing code already handle the case "variable is auto, but not in > current symbol table level". So I'm fine with your fixes to add this guarded > code to "SCLASS_FORMAL". > Sun, do you know who add these comments and bug number(634200, 555533) > before? > Regards. > Tianwei >> >> 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 >> > > > > -- > Sheng, Tianwei > Inst. of High Performance Computing > Dept. of Computer Sci. & Tech. > Tsinghua Univ. > > ------------------------------------------------------------------------------ > 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 > > ------------------------------------------------------------------------------ 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
