Yes, x is SCLASS_FORMAL. I meant the code treated x in the same way as a
local variable. By adding guarded code for SCLASS_FORMAL, x would be treated
in the same way as an uplevel auto variable.
Thanks.
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.
>
--
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
