It seems combining the code cleanup with bug fix is not a good practice. If
#ifdef KEY is not used anymore, we can initiate another round of code
cleanup to remove this macro.

2011/6/14 Sun Chan <sun.c...@gmail.com>

> 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
> >
> >
>
>
> ------------------------------------------------------------------------------
> 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
>



-- 
Regards,
Lai Jian-Xin
------------------------------------------------------------------------------
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