Hi, Sun
here is the new patch, I have remove the "#ifdef KEY" in the modify
function, and for the 2 new added function, I have make them as a
public member of CODEREP.
Could you please review it?
thanks very much.

On Tue, Jun 14, 2011 at 11:17 AM, Sun Chan <sun.c...@gmail.com> wrote:
> 1. WOPT does not allow static objects, can you put your new routines
> inside the right class?
> 2. please remove the #ifdef KEY, I think we can start removing this
> KEY thing now, everyone assumes this as default now
>
> Sun
>
> On Mon, Jun 13, 2011 at 7:52 PM, Wu Yongchong <wuyongch...@gmail.com> wrote:
>> Hi
>> can a gatekeeper help review this patch
>>
>> This patch tries to fix various C standard compliant problems related
>> to volatile in open64. Here is the list of the problems:
>>
>> 1. Incorrect simplification in the following case:
>>
>>  volatile int x;
>>  return x - x;
>>
>> The expression (x - x) cannot be simplified to 0 because x is volatile.
>>
>>  int v; volatile int val;
>>  val = *(volatile int*)(&v); /* should not be simplified to LDID v */
>>
>> According to C standard, the *(volatile int*)(&v) should be converted
>> into an ILOAD expression in WHIRL. However, the simplifier folds the
>> whole expression into LDID v.
>>
>> 2. Incorrect simplification related to pointers:
>>
>>  int test_volatile_ptrs1(){
>>    volatile int *p;
>>    int val;
>>
>>    val = p - p; /* can be folded to zero */
>>    val += (*p) - (*p); /* cannot be folded */
>>
>>    return val;
>>  }
>>
>> The expression (*p) - (*p) cannot be simplified since p is a pointer
>> to volatile int. However, WN simplifier fails to check for this.
>>
>> 3. Incorrect Boolean expression simplification
>>
>> The VHO lowering code tries to simplify certain Boolean operations
>> because of short circuiting in C99 semantics. However, it fails to
>> consider volatile in the case. For example: (x && 0) cannot be
>> simplified to 0 when (x) is a volatile.
>>
>> 4. Bugs related to aggregates that are volatile or have volatile members
>>
>>   int test_volatile_struct() {
>>     struct struct_typ {
>>       int foo;
>>       int bar;
>>     };
>>     struct vol_struct_typ {
>>       volatile int foo;
>>       volatile int bar;
>>     };
>>
>>     volatile struct struct_typ val;
>>     struct vol_struct_typ vol_val;
>>
>>     val = val; /* should not be deleted because val is volatile*/
>>     vol_val = vol_val; /* should not be deleted because its members
>> are volatile */
>>
>>     return 0;
>>   }
>>
>>   void test_volatile_partial_struct() {
>>     struct foo {
>>       volatile int bar;
>>       float baz;
>>     } v;
>>
>>     v = v; /* can be optimized to v.bar = v.bar only */
>>   }
>>
>>   void test_volatile_nested_struct() {
>>     struct foo {
>>       struct {
>>         volatile int val;
>>         int var;
>>       } bar;
>>       float baz;
>>     };
>>
>>     struct foo v;
>>     v = v;
>>   }
>>
>>   int test_volatile_arg_5(volatile struct foo_t *foop) {
>>     return foop->bar - foop->bar; /* should not be simplified to 0 */
>>   }
>>
>> In wn_lower.cxx, expression like struct_val = struct_val may have side
>> effects. Therefore, we need to traverse its fields and look for
>> volatile. The function is implemented as TY_has_volatile.
>>
>> WN_Is_Volatile_Mem checks for accessing volatile memories. However, it
>> only looks at WN_ty and WN_load_addr_ty. However, in the case of
>> aggregate type, the WN_ty does not reflect the type of the field being
>> accessed. We need to use WN_object_ty to get the type. However, adding
>> this code causes circular dependency between wn_util.h and wn_core.h.
>> The solution is to move WN_Is_Volatile_Mem to wn_util.h
>>
>> Failure to check for WN_object_ty also causes problems in WN
>> simplifier and WN_has_side_effects (which should return TRUE for
>> accessing a volatile field of a struct and FALSE otherwise).
>>
>>
>> --
>> yongchong
>>
>> ------------------------------------------------------------------------------
>> EditLive Enterprise is the world's most technically advanced content
>> authoring tool. Experience the power of Track Changes, Inline Image
>> Editing and ensure content is compliant with Accessibility Checking.
>> http://p.sf.net/sfu/ephox-dev2dev
>> _______________________________________________
>> Open64-devel mailing list
>> Open64-devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/open64-devel
>>
>>
>



-- 
yongchong

Attachment: volatile2.patch
Description: Binary data

------------------------------------------------------------------------------
EditLive Enterprise is the world's most technically advanced content
authoring tool. Experience the power of Track Changes, Inline Image
Editing and ensure content is compliant with Accessibility Checking.
http://p.sf.net/sfu/ephox-dev2dev
_______________________________________________
Open64-devel mailing list
Open64-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/open64-devel

Reply via email to