On Mon, Dec 20, 2010 at 7:25 PM, Jiangzhou HE <[email protected]> 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
> [email protected]
> 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
[email protected]
https://lists.sourceforge.net/lists/listinfo/open64-devel
