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

Attachment: volatile.diff
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