thx for volunteering. It certainly is a big step forward to easier reading code.
Sun
On Thu, Jun 16, 2011 at 1:33 AM, Jian-Xin Lai <laij...@gmail.com> wrote:
> 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
