Hi, yongchong: Thanks for the patch, I have collected your mail cases in the attached tarball.
I am not the gatekeeper. Due to my limitations, I get one question about
your patch,
==============================================================================
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.
===============================================================================
your suggest some changes in vho_lower.cxx:
Index: osprey/be/vho/vho_lower.cxx
===================================================================
--- osprey/be/vho/vho_lower.cxx (revision 3642)
+++ osprey/be/vho/vho_lower.cxx (working copy)
@@ -136,7 +136,9 @@
case OPR_ILOAD:
has_side_effects = TY_is_volatile(WN_ty(wn))
- || VHO_WN_has_side_effects ( WN_kid0(wn) );
+ || (TY_kind(WN_load_addr_ty(wn)) == KIND_POINTER
+ && TY_is_volatile(TY_pointed(WN_load_addr_ty(wn))))
+ || VHO_WN_has_side_effects ( WN_kid0(wn) );
break;
default:
@@ -2783,7 +2785,8 @@
else
if ( r_oper == OPR_INTCONST
- && WN_const_val(r_wn) == 0 ) {
+ && WN_const_val(r_wn) == 0
+ && !WN_has_side_effects(l_wn)) {
wn = r_wn;
simplified = TRUE;
@@ -2965,14 +2968,15 @@
I write a test case as:
1volatile int x;
2
3int f(void) {
4 return (x && 0);
5}
but I could not put the control to the suggest path.
Since, $TOOLROOT/bin/ir_b2a volatile_cand.B, I get
LOC 0 0 source files: 1
"/home1/yugang/work/bugs/volatile/volatile_cand.c"
FUNC_ENTRY <1,50,f> {line: 1/3}
BODY
BLOCK {line: 0/0}
END_BLOCK
BLOCK {line: 0/0}
END_BLOCK
BLOCK {line: 1/3}
PRAGMA 0 120 <null-st> 0 (0x0) # PREAMBLE_END {line: 1/3}
I4I4LDID 0 <1,51,x> T<4,.predef_I4,4,V>
EVAL {line: 1/4}
I4INTCONST 0 (0x0)
I4RETURN_VAL {line: 0/0}
END_BLOCK
And after wopt , I get
========== Driver dump after WOPT ==========
LOC 0 0 source files: 1
"/home1/yugang/work/bugs/volatile/volatile_cand.c"
FUNC_ENTRY <1,50,f> {line: 1/3}
BODY
BLOCK {line: 0/0}
END_BLOCK
BLOCK {line: 0/0}
END_BLOCK
BLOCK {line: 1/3}
PRAGMA 0 120 <null-st> 0 (0x0) # PREAMBLE_END {line: 1/3}
I4I4LDID 0 <1,51,x> T<4,.predef_I4,4,V>
EVAL {line: 0/0}
U4INTCONST 0 (0x0)
I4STID 1 <1,4,.preg_I4> T<4,.predef_I4,4> # $r1 {line: 0/0}
RETURN {line: 0/0}
END_BLOCK
you can also try:
volatile int x;
int y;
int f(void) {
return (x && 0) && y;
}
or
volatile int x;
int y;
int f(void) {
return y && (x && 0) ;
}
Am I missing something?
Thanks.
Gang
On Tue, Jun 14, 2011 at 10:52 AM, 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
>
>
volatile.tar.gz
Description: GNU Zip compressed 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
