Author: ycwu
Date: 2011-06-20 01:06:24 -0400 (Mon, 20 Jun 2011)
New Revision: 3653
Modified:
trunk/osprey/be/cg/whirl2ops.cxx
trunk/osprey/be/com/w2op.cxx
trunk/osprey/be/com/wn_lower.cxx
trunk/osprey/be/opt/opt_fold.cxx
trunk/osprey/be/opt/opt_fold.h
trunk/osprey/be/opt/opt_htable.h
trunk/osprey/be/vho/vho_lower.cxx
trunk/osprey/common/com/symtab.cxx
trunk/osprey/common/com/symtab_utils.h
trunk/osprey/common/com/wn.cxx
trunk/osprey/common/com/wn_core.h
trunk/osprey/common/com/wn_simp.cxx
trunk/osprey/common/com/wn_simp_code.h
trunk/osprey/common/com/wn_util.h
Log:
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).
C.R. Sun Chan
Modified: trunk/osprey/be/cg/whirl2ops.cxx
===================================================================
--- trunk/osprey/be/cg/whirl2ops.cxx 2011-06-17 19:59:39 UTC (rev 3652)
+++ trunk/osprey/be/cg/whirl2ops.cxx 2011-06-20 05:06:24 UTC (rev 3653)
@@ -995,7 +995,7 @@
{
OP *op;
// We don't have aliasing information at -O0 and -O1.
- if (CG_opt_level < 2) return;
+ if (CG_opt_level < 1) return;
op = Last_Mem_OP ? OP_next(Last_Mem_OP) : OPS_first(&New_OPs);
for ( ; op != NULL; op = OP_next(op)) {
Modified: trunk/osprey/be/com/w2op.cxx
===================================================================
--- trunk/osprey/be/com/w2op.cxx 2011-06-17 19:59:39 UTC (rev 3652)
+++ trunk/osprey/be/com/w2op.cxx 2011-06-20 05:06:24 UTC (rev 3653)
@@ -45,6 +45,7 @@
#include "topcode.h"
#include "targ_isa_properties.h"
#include "wn.h"
+#include "wn_util.h"
#include "opcode.h"
#include "config_targ.h"
#include "config.h"
Modified: trunk/osprey/be/com/wn_lower.cxx
===================================================================
--- trunk/osprey/be/com/wn_lower.cxx 2011-06-17 19:59:39 UTC (rev 3652)
+++ trunk/osprey/be/com/wn_lower.cxx 2011-06-20 05:06:24 UTC (rev 3653)
@@ -7456,7 +7456,9 @@
// Don't do struct copy if src and dest are the same.
if (WN_opcode(rhs) == OPC_MMLDID &&
WN_st(rhs) == WN_st(tree) &&
- WN_load_offset(rhs) == WN_store_offset(tree)) {
+ WN_load_offset(rhs) == WN_store_offset(tree) &&
+ !TY_is_volatile(ty_idx) &&
+ !TY_has_volatile(ty_idx)) {
WN_Delete(tree);
return NULL;
}
Modified: trunk/osprey/be/opt/opt_fold.cxx
===================================================================
--- trunk/osprey/be/opt/opt_fold.cxx 2011-06-17 19:59:39 UTC (rev 3652)
+++ trunk/osprey/be/opt/opt_fold.cxx 2011-06-20 05:06:24 UTC (rev 3653)
@@ -953,6 +953,51 @@
}
}
+TY_IDX
+CODEREP::lod_addr_ty()
+{
+ switch (Kind()) {
+ case CK_IVAR:
+ return CR_ty(Ilod_base());
+ default:
+ return TY_IDX_ZERO;
+ }
+}
+
+TY_IDX
+CODEREP::object_ty()
+{
+ switch (Kind()) {
+ case CK_VAR:
+ if (Field_id() != 0) {
+ UINT cur_field_id = 0;
+ TY_IDX ty_idx = Lod_ty();
+ UINT field_id = Field_id();
+ FLD_HANDLE fld = FLD_get_to_field (ty_idx, field_id, cur_field_id);
+ Is_True (!fld.Is_Null(), ("Invalid field id %d for type 0x%x",
+ field_id, ty_idx));
+ return FLD_type(fld);
+ } else {
+ return Lod_ty();
+ }
+ case CK_IVAR:
+ if (I_field_id() != 0) {
+ UINT cur_field_id = 0;
+ TY_IDX ty_idx = Ilod_ty();
+ UINT field_id = I_field_id();
+ FLD_HANDLE fld = FLD_get_to_field (ty_idx, field_id, cur_field_id);
+ Is_True (!fld.Is_Null(), ("Invalid field id %d for type 0x%x",
+ field_id, ty_idx));
+ return FLD_type(fld);
+ } else {
+ return Ilod_ty();
+ }
+
+ default:
+ return TY_IDX_ZERO;
+ }
+}
+
static TY_IDX
CR_ty(CODEREP *cr)
{
Modified: trunk/osprey/be/opt/opt_fold.h
===================================================================
--- trunk/osprey/be/opt/opt_fold.h 2011-06-17 19:59:39 UTC (rev 3652)
+++ trunk/osprey/be/opt/opt_fold.h 2011-06-20 05:06:24 UTC (rev 3653)
@@ -116,6 +116,8 @@
#define SIMPNODE_st CR_st
#define SIMPNODE_st_idx(x) ST_st_idx(CR_st(x))
#define SIMPNODE_ty CR_ty
+#define SIMPNODE_object_ty(x) (x)->object_ty()
+#define SIMPNODE_load_addr_ty(x) (x)->lod_addr_ty()
#define SIMPNODE_kid0 CR_kid0
// kid 1 means nothing to IVAR, only to OP
#define SIMPNODE_kid1(x) (((x)->Kind() == CK_OP) ? \
@@ -139,7 +141,7 @@
#define SIMPNODE_i_bit_offset(x) (x)->I_bit_offset()
#define SIMPNODE_op_bit_offset(x) (x)->Op_bit_offset()
#define SIMPNODE_op_bit_size(x) (x)->Op_bit_size()
-
+#define SIMPNODE_is_volatile(x) (x)->Is_var_volatile()
/* on/off switch and trace file */
#define SIMPNODE_enable WOPT_Enable_CRSIMP
#define TRACEFILE TFile
Modified: trunk/osprey/be/opt/opt_htable.h
===================================================================
--- trunk/osprey/be/opt/opt_htable.h 2011-06-17 19:59:39 UTC (rev 3652)
+++ trunk/osprey/be/opt/opt_htable.h 2011-06-20 05:06:24 UTC (rev 3653)
@@ -1312,6 +1312,9 @@
BB_NODE* Get_ISOP_def_before_use_cache () const
{ return (BB_NODE*) u2.isop.node_cache; }
+
+ TY_IDX lod_addr_ty(void);
+ TY_IDX object_ty(void);
}; // end of class CODEREP
// Functions to tell how much extra space should be allocated with a
Modified: trunk/osprey/be/vho/vho_lower.cxx
===================================================================
--- trunk/osprey/be/vho/vho_lower.cxx 2011-06-17 19:59:39 UTC (rev 3652)
+++ trunk/osprey/be/vho/vho_lower.cxx 2011-06-20 05:06:24 UTC (rev 3653)
@@ -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 @@
else
if ( r_oper == OPR_INTCONST
- && WN_const_val(r_wn) == 1 ) {
+ && WN_const_val(r_wn) == 1
+ && !WN_has_side_effects(l_wn)) {
wn = r_wn;
simplified = TRUE;
}
/* simplify
- * ( e && FALSE )
+ * ( e || FALSE )
* to
* ( e )
*/
@@ -4457,7 +4461,8 @@
WN_kid0(wn) = vho_lower_expr (WN_kid0(wn), block, NULL);
if ( VHO_Iload_Opt
- && WN_operator(WN_kid0(wn)) == OPR_LDA ) {
+ && WN_operator(WN_kid0(wn)) == OPR_LDA
+ && !VHO_WN_has_side_effects(wn)) {
INT64 offset;
Modified: trunk/osprey/common/com/symtab.cxx
===================================================================
--- trunk/osprey/common/com/symtab.cxx 2011-06-17 19:59:39 UTC (rev 3652)
+++ trunk/osprey/common/com/symtab.cxx 2011-06-20 05:06:24 UTC (rev 3653)
@@ -1229,6 +1229,28 @@
return FALSE;
}
+/* ty has volatile flag in any of its members */
+BOOL
+TY_has_volatile (TY_IDX ty)
+{
+ if (TY_is_volatile(ty)) {
+ return TRUE;
+ }
+ if (TY_kind(ty) != KIND_STRUCT) {
+ return FALSE;
+ }
+
+ FLD_HANDLE fld = TY_fld(ty);
+ do {
+ TY_IDX fty = FLD_type(fld);
+ if (TY_has_volatile(fty)) {
+ return TRUE;
+ }
+ fld = FLD_next (fld);
+ } while (!fld.Is_Null());
+ return FALSE;
+}
+
#ifdef TARG_NVISA
/* number of elements in the vector */
UINT
Modified: trunk/osprey/common/com/symtab_utils.h
===================================================================
--- trunk/osprey/common/com/symtab_utils.h 2011-06-17 19:59:39 UTC (rev
3652)
+++ trunk/osprey/common/com/symtab_utils.h 2011-06-20 05:06:24 UTC (rev
3653)
@@ -201,6 +201,9 @@
/* ty either is union or has union in one of its fields (called recursively) */
BOOL TY_has_union (TY_IDX ty);
+/* ty is an aggregate type that has some member of volatile type */
+BOOL TY_has_volatile (TY_IDX ty);
+
#ifdef TARG_NVISA
/* base ty (field ty) of vector */
inline TY_IDX
Modified: trunk/osprey/common/com/wn.cxx
===================================================================
--- trunk/osprey/common/com/wn.cxx 2011-06-17 19:59:39 UTC (rev 3652)
+++ trunk/osprey/common/com/wn.cxx 2011-06-20 05:06:24 UTC (rev 3653)
@@ -81,9 +81,7 @@
#include "tracing.h"
#endif /* BACK_END */
-#if (defined(FRONT_END_C) || defined(FRONT_END_CPLUSPLUS)) &&
!defined(FRONT_END_MFEF77)
#include "wn_util.h"
-#endif /* (defined(FRONT_END_C) || defined(FRONT_END_CPLUSPLUS)) &&
!defined(FRONT_END_MFEF77) */
#if !defined(FRONT_END_C) && !defined(IR_TOOLS) && defined(TARG_SL)
#include "intrn_info.h"
#endif
@@ -3349,7 +3347,8 @@
case OPR_LDID:
case OPR_LDBITS:
- if (TY_is_volatile (WN_ty(wn)))
+ if (TY_is_volatile (WN_ty(wn)) ||
+ TY_is_volatile(WN_object_ty(wn)))
return (TRUE);
return FALSE;
Modified: trunk/osprey/common/com/wn_core.h
===================================================================
--- trunk/osprey/common/com/wn_core.h 2011-06-17 19:59:39 UTC (rev 3652)
+++ trunk/osprey/common/com/wn_core.h 2011-06-20 05:06:24 UTC (rev 3653)
@@ -1366,28 +1366,7 @@
/*REFERENCED*/
-inline BOOL WN_Is_Volatile_Mem(const WN *wn)
-{
- OPCODE opc = WN_opcode(wn);
- if (OPCODE_has_1ty(opc) || OPCODE_has_2ty(opc)) {
- if (OPCODE_operator(opc) == OPR_ISTORE ||
- OPCODE_operator(opc) == OPR_MSTORE) {
- TY_IDX pointed = TY_pointed (Ty_Table[WN_ty (wn)]);
- DevAssert(pointed, ("TY_pointed of ISTORE/MSTORE type is NULL"));
- return TY_is_volatile(pointed);
- } else {
- return TY_is_volatile(WN_ty(wn)) ||
-#ifdef KEY // bug 12404
- OPCODE_has_2ty(opc) && TY_is_volatile(TY_pointed(WN_load_addr_ty(wn)));
-#else
- OPCODE_has_2ty(opc) && TY_is_volatile(WN_load_addr_ty(wn));
-#endif
- }
- }
- return FALSE;
-}
-
#define WN_IF_IS_GUARD 0x1
#define WN_Is_If_Guard(x) (WN_if_flag(x) & WN_IF_IS_GUARD)
#define WN_Set_If_Guard(x) (WN_if_flag(x) |= WN_IF_IS_GUARD)
Modified: trunk/osprey/common/com/wn_simp.cxx
===================================================================
--- trunk/osprey/common/com/wn_simp.cxx 2011-06-17 19:59:39 UTC (rev 3652)
+++ trunk/osprey/common/com/wn_simp.cxx 2011-06-20 05:06:24 UTC (rev 3653)
@@ -113,6 +113,8 @@
#define SIMPNODE_st WN_st
#define SIMPNODE_st_idx WN_st_idx
#define SIMPNODE_ty WN_ty
+#define SIMPNODE_object_ty WN_object_ty
+#define SIMPNODE_load_addr_ty WN_load_addr_ty
#define SIMPNODE_kid0 WN_kid0
#define SIMPNODE_kid1 WN_kid1
#define SIMPNODE_kid WN_kid
@@ -155,7 +157,7 @@
#endif
#define SIMPNODE_Simplify_Initialize WN_Simplify_Initialize
#define SIMPNODE_Compare_Symbols WN_Compare_Symbols
-
+#define SIMPNODE_is_volatile WN_Is_Volatile_Mem
/* externally visible routines. These three are defined in wn_simp_code.h.
* They need a name defined here and in whatever external interface file
* exists for the routine
Modified: trunk/osprey/common/com/wn_simp_code.h
===================================================================
--- trunk/osprey/common/com/wn_simp_code.h 2011-06-17 19:59:39 UTC (rev
3652)
+++ trunk/osprey/common/com/wn_simp_code.h 2011-06-20 05:06:24 UTC (rev
3653)
@@ -935,15 +935,35 @@
return SIMPNODE_Compare_Symbols(t1,t2);
case OPR_ILOAD:
+ {
+ int ret;
+
if (SIMPNODE_load_offset(t1) < SIMPNODE_load_offset(t2)) return(-1);
if (SIMPNODE_load_offset(t1) > SIMPNODE_load_offset(t2)) return(1);
if (SIMPNODE_desc(t1) == MTYPE_BS || SIMPNODE_desc(t2) == MTYPE_BS) {
- if (SIMPNODE_i_field_id(t1) < SIMPNODE_i_field_id(t2)) return(-1);
- if (SIMPNODE_i_field_id(t1) > SIMPNODE_i_field_id(t2)) return(1);
+ if (SIMPNODE_i_field_id(t1) < SIMPNODE_i_field_id(t2)) return(-1);
+ if (SIMPNODE_i_field_id(t1) > SIMPNODE_i_field_id(t2)) return(1);
}
- return (SIMPNODE_Simp_Compare_Trees(SIMPNODE_kid0(t1),
- SIMPNODE_kid0(t2)));
+ ret = SIMPNODE_Simp_Compare_Trees(SIMPNODE_kid0(t1),SIMPNODE_kid0(t2));
+ if (ret == 0) {
+ BOOL ty_is_volatile ;
+ BOOL lod_addr_volatile ;
+ BOOL item_is_volatile ;
+ ty_is_volatile = TY_is_volatile(SIMPNODE_ty(t1))
+ || TY_is_volatile(SIMPNODE_ty(t2));
+ lod_addr_volatile =
TY_is_volatile(TY_pointed(SIMPNODE_load_addr_ty(t1)))
+ || TY_is_volatile(TY_pointed(SIMPNODE_load_addr_ty(t2)));
+ item_is_volatile = TY_is_volatile(SIMPNODE_object_ty(t1))
+ || TY_is_volatile(SIMPNODE_object_ty(t2));
+
+ if (ty_is_volatile || lod_addr_volatile || item_is_volatile) {
+ return t1 - t2;
+ }
+ }
+ return ret;
+ }
+
case OPR_ILDBITS:
if (SIMPNODE_load_offset(t1) < SIMPNODE_load_offset(t2)) return(-1);
if (SIMPNODE_load_offset(t1) > SIMPNODE_load_offset(t2)) return(1);
@@ -962,14 +982,34 @@
SIMPNODE_kid1(t2)));
case OPR_LDID:
- if (SIMPNODE_load_offset(t1) < SIMPNODE_load_offset(t2)) return(-1);
- if (SIMPNODE_load_offset(t1) > SIMPNODE_load_offset(t2)) return(1);
- if (SIMPNODE_desc(t1) == MTYPE_BS || SIMPNODE_desc(t2) == MTYPE_BS) {
- if (SIMPNODE_field_id(t1) < SIMPNODE_field_id(t2)) return(-1);
- if (SIMPNODE_field_id(t1) > SIMPNODE_field_id(t2)) return(1);
- }
- return SIMPNODE_Compare_Symbols(t1,t2);
+ {
+ int ret;
+ if (SIMPNODE_load_offset(t1) < SIMPNODE_load_offset(t2)) return(-1);
+ if (SIMPNODE_load_offset(t1) > SIMPNODE_load_offset(t2)) return(1);
+ if (SIMPNODE_desc(t1) == MTYPE_BS || SIMPNODE_desc(t2) == MTYPE_BS) {
+ if (SIMPNODE_field_id(t1) < SIMPNODE_field_id(t2)) return(-1);
+ if (SIMPNODE_field_id(t1) > SIMPNODE_field_id(t2)) return(1);
+ }
+ ret = SIMPNODE_Compare_Symbols(t1,t2);
+ if (ret == 0){
+ BOOL ty_is_volatile = TY_is_volatile(SIMPNODE_ty(t1)) ||
TY_is_volatile(SIMPNODE_ty(t2));
+ if (ty_is_volatile) {
+ return t1 - t2;
+ }
+ if (SIMPNODE_field_id(t1) || SIMPNODE_field_id(t2)) {
+ TY_IDX t1_obj_ty = SIMPNODE_object_ty(t1);
+ TY_IDX t2_obj_ty = SIMPNODE_object_ty(t2);
+
+ ty_is_volatile = TY_is_volatile(t1_obj_ty) ||
TY_is_volatile(t2_obj_ty);
+ if (ty_is_volatile) {
+ return t1 - t2;
+ }
+ }
+ }
+ return ret;
+ }
+
case OPR_LDBITS:
if (SIMPNODE_load_offset(t1) < SIMPNODE_load_offset(t2)) return(-1);
if (SIMPNODE_load_offset(t1) > SIMPNODE_load_offset(t2)) return(1);
@@ -5768,6 +5808,11 @@
}
}
+ if (SIMPNODE_is_volatile(k0)
+ || SIMPNODE_is_volatile(k1)) {
+ return result;
+ }
+
if (Simp_Canonicalize) {
/* Simply canonicalization for constants. *******************************
c1 op1 x x op2 c2 if op1 is one of the
following:
@@ -6573,6 +6618,10 @@
if (!SIMPNODE_enable || !WN_Simp_Fold_ILOAD) return (r);
+ if (TY_is_volatile(ty)
+ || (TY_kind(load_addr_ty) == KIND_POINTER &&
TY_is_volatile(TY_pointed(load_addr_ty))))
+ return (r);
+
if (!SIMPNODE_simp_initialized) SIMPNODE_Simplify_Initialize();
#ifdef KEY
/* Look for ILOAD(LDA) of constants in memory */
Modified: trunk/osprey/common/com/wn_util.h
===================================================================
--- trunk/osprey/common/com/wn_util.h 2011-06-17 19:59:39 UTC (rev 3652)
+++ trunk/osprey/common/com/wn_util.h 2011-06-20 05:06:24 UTC (rev 3653)
@@ -445,6 +445,24 @@
/* Obtain the high-level type of the item accessed */
extern TY_IDX WN_object_ty(const WN *);
+inline BOOL WN_Is_Volatile_Mem(const WN *wn)
+{
+ OPCODE opc = WN_opcode(wn);
+ if (OPCODE_has_1ty(opc) || OPCODE_has_2ty(opc)) {
+ if (OPCODE_operator(opc) == OPR_ISTORE ||
+ OPCODE_operator(opc) == OPR_MSTORE) {
+ TY_IDX pointed = TY_pointed (Ty_Table[WN_ty (wn)]);
+ DevAssert(pointed, ("TY_pointed of ISTORE/MSTORE type is NULL"));
+ return TY_is_volatile(pointed);
+ } else {
+ return TY_is_volatile(WN_ty(wn)) ||
+ TY_is_volatile(WN_object_ty(wn)) ||
+ (OPCODE_has_2ty(opc) &&
TY_is_volatile(TY_pointed(WN_load_addr_ty(wn))));
+ }
+ }
+ return FALSE;
+}
+
/* Obtain the higher level type of object being accessed. */
extern void WN_hl_object_ty (const WN*, TY_IDX& ty, UINT32& fld_id);
------------------------------------------------------------------------------
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
