It seems we always need a round of code cleanup because the bug fixes can
only touch very small piece of the code. Maybe one big change to remove all
KEY is much easier to both developers and gate keepers. If there is no other
volunteers, we can take this.
2011/6/16 Sun Chan <sun.c...@gmail.com>
> Does this mean you are volunteering? I suspect the only sure way
> (although slow) is simply remove them while people fixing/changing
> things. Else, it can only get worst
> Sun
>
> On Wed, Jun 15, 2011 at 9:59 PM, Jian-Xin Lai <laij...@gmail.com> wrote:
> > 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
> >
>
--
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
