[Open64-devel] Code review for bug1011[WOPT]

[zhangliwei]

Hi,

Could someone review the patch for Bug1011? Thanks very much.



This solution is that when the compiler improves the variable "a" alignment
from 1byte to 4byte, the derived variable "a.init" alignment also change
from 1 to 4.



This is the patch:

1 Index: be/com/wn_lower.cxx

2 ===================================================================

3 --- be/com/wn_lower.cxx (revision 4043)

4 +++ be/com/wn_lower.cxx (working copy)

5 @@ -9513,6 +9513,14 @@

6 {

7 symType = Make_Pointer_Type(newType);

8 }

9 + ST *sym_init = Find_Derived_Symbol(ST_name(sym));

10 + if(sym_init)

11 + {

12 + FmtAssert((ST_type(sym_init) == ST_type(sym) ||
ST_type(sym_init) == symType) ,

13 + ("variable %s and its deriveed symble %s have different
type.",

14 + ST_name(sym), ST_name(sym_init)));

15 + Set_ST_type(sym_init, symType);

16 + }

17 Set_ST_type(sym, symType);

18

19 if (stAlign > align)

20 Index: kgccfe/wfe_decl.cxx

21 ===================================================================

22 --- kgccfe/wfe_decl.cxx (revision 4043)

23 +++ kgccfe/wfe_decl.cxx (working copy)

24 @@ -2257,15 +2257,7 @@

25 WFE_Generate_Temp_For_Initialized_Aggregate (tree init, char * name)

26 {

27 TY_IDX ty_idx = Get_TY(TREE_TYPE(init));

28 - ST *temp = New_ST (CURRENT_SYMTAB);

29 - ST_Init (temp,

30 -#ifdef TARG_NVISA

31 - Save_Str2 (name, "_init"),

32 -#else

33 - Save_Str2 (name, ".init"),

34 -#endif

35 - CLASS_VAR, SCLASS_PSTATIC, EXPORT_LOCAL,

36 - ty_idx );

37 + ST *temp = Make_Derived_Symbol(name,ty_idx);

38 if (TREE_CODE(init) == CONSTRUCTOR

39 && ! Use_Static_Init_For_Aggregate (temp, init))

40 {

41 Index: common/com/symtab.h

42 ===================================================================

43 --- common/com/symtab.h (revision 4043)

44 +++ common/com/symtab.h (working copy)

45 @@ -232,6 +232,33 @@

46 && (ST_strong(s) != s);

47 }

48

49 +inline ST *

50 +Make_Derived_Symbol(char * name, TY_IDX ty_idx){

51 + ST *temp = New_ST(CURRENT_SYMTAB);

52 + ST_Init(temp,

53 + Save_Str2(name, ST_INIT_SUFFIX),

54 + CLASS_VAR, SCLASS_PSTATIC, EXPORT_LOCAL,

55 + ty_idx);

56 + return temp;

57 +}

58 +

59 +inline ST *

60 +Find_Derived_Symbol(char * name){

61 + ST * sym_init = NULL;

62 + int i = 0;

63 + int len = strlen(name);

64 + FOREACH_SYMBOL(CURRENT_SYMTAB,sym_init,i)

65 + {

66 + char *p= ST_name(ST_st_idx(sym_init));

67 + if(p && memcmp(p, name, len) == 0 &&

68 + memcmp(p+len, ST_INIT_SUFFIX, ST_INIT_SUFFIX_LEN) == 0)

69 + {

70 + return sym_init;

71 + }

72 + }

73 + return NULL;

74 +}

75 +

76
//----------------------------------------------------------------------

77 // INITO

78
//----------------------------------------------------------------------

79 Index: common/com/symtab_defs.h

80 ===================================================================

81 --- common/com/symtab_defs.h (revision 4043)

82 +++ common/com/symtab_defs.h (working copy)

83 @@ -1127,5 +1127,11 @@

84 typedef SYMTAB_HEADER_TABLE<GLOBAL_SYMTAB_TABLES>
GLOBAL_SYMTAB_HEADER_TABLE;

85 typedef SYMTAB_HEADER_TABLE<LOCAL_SYMTAB_TABLES>
LOCAL_SYMTAB_HEADER_TABLE;

86

87 +#ifdef TARG_NVISA

88 +#define ST_INIT_SUFFIX "_init"

89 +#else

90 +#define ST_INIT_SUFFIX ".init"

91 +#endif

92 +#define ST_INIT_SUFFIX_LEN sizeof(ST_INIT_SUFFIX)

93 #endif /* symtab_defs_INCLUDED */

94

95 Index: wgen/wgen_decl.cxx

96 ===================================================================

97 --- wgen/wgen_decl.cxx (revision 4043)

98 +++ wgen/wgen_decl.cxx (working copy)

99 @@ -4452,11 +4452,7 @@

100 WGEN_Generate_Temp_For_Initialized_Aggregate (gs_t init, char * name)

101 {

102 TY_IDX ty_idx = Get_TY(gs_tree_type(init));

103 - ST *temp = New_ST (CURRENT_SYMTAB);

104 - ST_Init (temp,

105 - Save_Str2 (name, ".init"),

106 - CLASS_VAR, SCLASS_PSTATIC, EXPORT_LOCAL,

107 - ty_idx );

108 + ST * temp = Make_Derived_Symbol(name,ty_idx);

109 AGGINIT agginit;

110 gs_code_t code = gs_tree_code(init);

111 if (code == GS_CONSTRUCTOR

112 @@ -4546,11 +4542,7 @@

113 DevWarn ("Static initialize %s(%s)\n",

114 ST_name(target_st),
Sclass_Name(ST_sclass(target_st)));

115 TY_IDX ty_idx = Get_TY(gs_tree_type(init));

116 - ST *temp = New_ST (CURRENT_SYMTAB);

117 - ST_Init (temp,

118 - Save_Str2 (ST_name(target_st), ".init"),

119 - CLASS_VAR, SCLASS_PSTATIC, EXPORT_LOCAL,

120 - ty_idx );

121 + ST *temp = Make_Derived_Symbol(ST_name(target_st),ty_idx);

122 // setup inito for target_st

123 Set_ST_is_initialized(temp);

124 agginit.Set_inito(New_INITO(temp));

125 Index: kg++fe/wfe_decl.cxx

126 ===================================================================

127 --- kg++fe/wfe_decl.cxx (revision 4043)

128 +++ kg++fe/wfe_decl.cxx (working copy)

129 @@ -3113,11 +3113,7 @@

130 WFE_Generate_Temp_For_Initialized_Aggregate (tree init, char * name)

131 {

132 TY_IDX ty_idx = Get_TY(TREE_TYPE(init));

133 - ST *temp = New_ST (CURRENT_SYMTAB);

134 - ST_Init (temp,

135 - Save_Str2 (name, ".init"),

136 - CLASS_VAR, SCLASS_PSTATIC, EXPORT_LOCAL,

137 - ty_idx );

138 + ST *temp = Make_Derived_Symbol(name,ty_idx);

139 if (TREE_CODE(init) == CONSTRUCTOR

140 && ! Use_Static_Init_For_Aggregate (temp, init))

141 {

142 @@ -3172,11 +3168,7 @@

143 DevWarn ("Static initialize %s(%s)\n",

144 ST_name(target_st),
Sclass_Name(ST_sclass(target_st)));

145 TY_IDX ty_idx = Get_TY(TREE_TYPE(init));

146 - ST *temp = New_ST (CURRENT_SYMTAB);

147 - ST_Init (temp,

148 - Save_Str2 (ST_name(target_st), ".init"),

149 - CLASS_VAR, SCLASS_PSTATIC, EXPORT_LOCAL,

150 - ty_idx );

151 + ST *temp = Make_Derived_Symbol(name,ty_idx);

152 // setup inito for target_st

153 Set_ST_is_initialized(temp);

154 aggregate_inito = New_INITO (temp);



Best wishes,

Liwei Zhang

[Sun Chan]

are you sure you want to inline Find_derived_symbol?
Sun

Post by zhangliwei
Hi,
Could someone review the patch for Bug1011? Thanks very much.
This solution is that when the compiler improves the variable “a” alignment
from 1byte to 4byte, the derived variable “a.init” alignment also change
from 1 to 4.
1 Index: be/com/wn_lower.cxx
2 ===================================================================
3 --- be/com/wn_lower.cxx (revision 4043)
4 +++ be/com/wn_lower.cxx (working copy)
6 {
7 symType = Make_Pointer_Type(newType);
8 }
9 + ST *sym_init = Find_Derived_Symbol(ST_name(sym));
10 + if(sym_init)
11 + {
12 + FmtAssert((ST_type(sym_init) == ST_type(sym) ||
ST_type(sym_init) == symType) ,
13 + ("variable %s and its deriveed symble %s have different
type.",
14 + ST_name(sym), ST_name(sym_init)));
15 + Set_ST_type(sym_init, symType);
16 + }
17 Set_ST_type(sym, symType);
18
19 if (stAlign > align)
20 Index: kgccfe/wfe_decl.cxx
21 ===================================================================
22 --- kgccfe/wfe_decl.cxx (revision 4043)
23 +++ kgccfe/wfe_decl.cxx (working copy)
25 WFE_Generate_Temp_For_Initialized_Aggregate (tree init, char * name)
26 {
27 TY_IDX ty_idx = Get_TY(TREE_TYPE(init));
28 - ST *temp = New_ST (CURRENT_SYMTAB);
29 - ST_Init (temp,
30 -#ifdef TARG_NVISA
31 - Save_Str2 (name, "_init"),
32 -#else
33 - Save_Str2 (name, ".init"),
34 -#endif
35 - CLASS_VAR, SCLASS_PSTATIC, EXPORT_LOCAL,
36 - ty_idx );
37 + ST *temp = Make_Derived_Symbol(name,ty_idx);
38 if (TREE_CODE(init) == CONSTRUCTOR
39 && ! Use_Static_Init_For_Aggregate (temp, init))
40 {
41 Index: common/com/symtab.h
42 ===================================================================
43 --- common/com/symtab.h (revision 4043)
44 +++ common/com/symtab.h (working copy)
46 && (ST_strong(s) != s);
47 }
48
49 +inline ST *
50 +Make_Derived_Symbol(char * name, TY_IDX ty_idx){
51 + ST *temp = New_ST(CURRENT_SYMTAB);
52 + ST_Init(temp,
53 + Save_Str2(name, ST_INIT_SUFFIX),
54 + CLASS_VAR, SCLASS_PSTATIC, EXPORT_LOCAL,
55 + ty_idx);
56 + return temp;
57 +}
58 +
59 +inline ST *
60 +Find_Derived_Symbol(char * name){
61 + ST * sym_init = NULL;
62 + int i = 0;
63 + int len = strlen(name);
64 + FOREACH_SYMBOL(CURRENT_SYMTAB,sym_init,i)
65 + {
66 + char *p= ST_name(ST_st_idx(sym_init));
67 + if(p && memcmp(p, name, len) == 0 &&
68 + memcmp(p+len, ST_INIT_SUFFIX, ST_INIT_SUFFIX_LEN) == 0)
69 + {
70 + return sym_init;
71 + }
72 + }
73 + return NULL;
74 +}
75 +
76
//----------------------------------------------------------------------
77 // INITO
78
//----------------------------------------------------------------------
79 Index: common/com/symtab_defs.h
80 ===================================================================
81 --- common/com/symtab_defs.h (revision 4043)
82 +++ common/com/symtab_defs.h (working copy)
84 typedef SYMTAB_HEADER_TABLE<GLOBAL_SYMTAB_TABLES>
GLOBAL_SYMTAB_HEADER_TABLE;
85 typedef SYMTAB_HEADER_TABLE<LOCAL_SYMTAB_TABLES>
LOCAL_SYMTAB_HEADER_TABLE;
86
87 +#ifdef TARG_NVISA
88 +#define ST_INIT_SUFFIX "_init"
89 +#else
90 +#define ST_INIT_SUFFIX ".init"
91 +#endif
92 +#define ST_INIT_SUFFIX_LEN sizeof(ST_INIT_SUFFIX)
93 #endif /* symtab_defs_INCLUDED */
94
95 Index: wgen/wgen_decl.cxx
96 ===================================================================
97 --- wgen/wgen_decl.cxx (revision 4043)
98 +++ wgen/wgen_decl.cxx (working copy)
100 WGEN_Generate_Temp_For_Initialized_Aggregate (gs_t init, char * name)
101 {
102 TY_IDX ty_idx = Get_TY(gs_tree_type(init));
103 - ST *temp = New_ST (CURRENT_SYMTAB);
104 - ST_Init (temp,
105 - Save_Str2 (name, ".init"),
106 - CLASS_VAR, SCLASS_PSTATIC, EXPORT_LOCAL,
107 - ty_idx );
108 + ST * temp = Make_Derived_Symbol(name,ty_idx);
109 AGGINIT agginit;
110 gs_code_t code = gs_tree_code(init);
111 if (code == GS_CONSTRUCTOR
113 DevWarn ("Static initialize %s(%s)\n",
114 ST_name(target_st),
Sclass_Name(ST_sclass(target_st)));
115 TY_IDX ty_idx = Get_TY(gs_tree_type(init));
116 - ST *temp = New_ST (CURRENT_SYMTAB);
117 - ST_Init (temp,
118 - Save_Str2 (ST_name(target_st), ".init"),
119 - CLASS_VAR, SCLASS_PSTATIC, EXPORT_LOCAL,
120 - ty_idx );
121 + ST *temp = Make_Derived_Symbol(ST_name(target_st),ty_idx);
122 // setup inito for target_st
123 Set_ST_is_initialized(temp);
124 agginit.Set_inito(New_INITO(temp));
125 Index: kg++fe/wfe_decl.cxx
126 ===================================================================
127 --- kg++fe/wfe_decl.cxx (revision 4043)
128 +++ kg++fe/wfe_decl.cxx (working copy)
130 WFE_Generate_Temp_For_Initialized_Aggregate (tree init, char * name)
131 {
132 TY_IDX ty_idx = Get_TY(TREE_TYPE(init));
133 - ST *temp = New_ST (CURRENT_SYMTAB);
134 - ST_Init (temp,
135 - Save_Str2 (name, ".init"),
136 - CLASS_VAR, SCLASS_PSTATIC, EXPORT_LOCAL,
137 - ty_idx );
138 + ST *temp = Make_Derived_Symbol(name,ty_idx);
139 if (TREE_CODE(init) == CONSTRUCTOR
140 && ! Use_Static_Init_For_Aggregate (temp, init))
141 {
143 DevWarn ("Static initialize %s(%s)\n",
144 ST_name(target_st),
Sclass_Name(ST_sclass(target_st)));
145 TY_IDX ty_idx = Get_TY(TREE_TYPE(init));
146 - ST *temp = New_ST (CURRENT_SYMTAB);
147 - ST_Init (temp,
148 - Save_Str2 (ST_name(target_st), ".init"),
149 - CLASS_VAR, SCLASS_PSTATIC, EXPORT_LOCAL,
150 - ty_idx );
151 + ST *temp = Make_Derived_Symbol(name,ty_idx);
152 // setup inito for target_st
153 Set_ST_is_initialized(temp);
154 aggregate_inito = New_INITO (temp);
Best wishes,
Liwei Zhang
------------------------------------------------------------------------------
INSIGHTS What's next for parallel hardware, programming and related areas?
Interviews and blogs by thought leaders keep you ahead of the curve.
http://goparallel.sourceforge.net
_______________________________________________
Open64-devel mailing list
https://lists.sourceforge.net/lists/listinfo/open64-devel

[zhangliwei]

[Sun Chan]

so may be those new functions doesn't belong in the .h file
Sun

Hi, Sun
You are right.
This function is a little complex , "inline" is not suitable.
But all other functions in this file is inline, this function will looks
inconsistent.
Which method will be better?
Thanks.
Liwei Zhang
-----邮件原件-----
发送时间: 2012年11月30日 7:05
收件人: zhangliwei
主题: Re: [Open64-devel] Code review for bug1011[WOPT]
are you sure you want to inline Find_derived_symbol?
Sun

Post by zhangliwei
Hi,
Could someone review the patch for Bug1011? Thanks very much.
This solution is that when the compiler improves the variable “a”

alignment

Post by zhangliwei
from 1byte to 4byte, the derived variable “a.init” alignment also change
from 1 to 4.
1 Index: be/com/wn_lower.cxx
2 ===================================================================
3 --- be/com/wn_lower.cxx (revision 4043)
4 +++ be/com/wn_lower.cxx (working copy)
6 {
7 symType = Make_Pointer_Type(newType);
8 }
9 + ST *sym_init = Find_Derived_Symbol(ST_name(sym));
10 + if(sym_init)
11 + {
12 + FmtAssert((ST_type(sym_init) == ST_type(sym) ||
ST_type(sym_init) == symType) ,
13 + ("variable %s and its deriveed symble %s have different
type.",
14 + ST_name(sym), ST_name(sym_init)));
15 + Set_ST_type(sym_init, symType);
16 + }
17 Set_ST_type(sym, symType);
18
19 if (stAlign > align)
20 Index: kgccfe/wfe_decl.cxx
21 ===================================================================
22 --- kgccfe/wfe_decl.cxx (revision 4043)
23 +++ kgccfe/wfe_decl.cxx (working copy)
25 WFE_Generate_Temp_For_Initialized_Aggregate (tree init, char * name)
26 {
27 TY_IDX ty_idx = Get_TY(TREE_TYPE(init));
28 - ST *temp = New_ST (CURRENT_SYMTAB);
29 - ST_Init (temp,
30 -#ifdef TARG_NVISA
31 - Save_Str2 (name, "_init"),
32 -#else
33 - Save_Str2 (name, ".init"),
34 -#endif
35 - CLASS_VAR, SCLASS_PSTATIC, EXPORT_LOCAL,
36 - ty_idx );
37 + ST *temp = Make_Derived_Symbol(name,ty_idx);
38 if (TREE_CODE(init) == CONSTRUCTOR
39 && ! Use_Static_Init_For_Aggregate (temp, init))
40 {
41 Index: common/com/symtab.h
42 ===================================================================
43 --- common/com/symtab.h (revision 4043)
44 +++ common/com/symtab.h (working copy)
46 && (ST_strong(s) != s);
47 }
48
49 +inline ST *
50 +Make_Derived_Symbol(char * name, TY_IDX ty_idx){
51 + ST *temp = New_ST(CURRENT_SYMTAB);
52 + ST_Init(temp,
53 + Save_Str2(name, ST_INIT_SUFFIX),
54 + CLASS_VAR, SCLASS_PSTATIC, EXPORT_LOCAL,
55 + ty_idx);
56 + return temp;
57 +}
58 +
59 +inline ST *
60 +Find_Derived_Symbol(char * name){
61 + ST * sym_init = NULL;
62 + int i = 0;
63 + int len = strlen(name);
64 + FOREACH_SYMBOL(CURRENT_SYMTAB,sym_init,i)
65 + {
66 + char *p= ST_name(ST_st_idx(sym_init));
67 + if(p && memcmp(p, name, len) == 0 &&
68 + memcmp(p+len, ST_INIT_SUFFIX, ST_INIT_SUFFIX_LEN) == 0)
69 + {
70 + return sym_init;
71 + }
72 + }
73 + return NULL;
74 +}
75 +
76
//----------------------------------------------------------------------
77 // INITO
78
//----------------------------------------------------------------------
79 Index: common/com/symtab_defs.h
80 ===================================================================
81 --- common/com/symtab_defs.h (revision 4043)
82 +++ common/com/symtab_defs.h (working copy)
84 typedef SYMTAB_HEADER_TABLE<GLOBAL_SYMTAB_TABLES>
GLOBAL_SYMTAB_HEADER_TABLE;
85 typedef SYMTAB_HEADER_TABLE<LOCAL_SYMTAB_TABLES>
LOCAL_SYMTAB_HEADER_TABLE;
86
87 +#ifdef TARG_NVISA
88 +#define ST_INIT_SUFFIX "_init"
89 +#else
90 +#define ST_INIT_SUFFIX ".init"
91 +#endif
92 +#define ST_INIT_SUFFIX_LEN sizeof(ST_INIT_SUFFIX)
93 #endif /* symtab_defs_INCLUDED */
94
95 Index: wgen/wgen_decl.cxx
96 ===================================================================
97 --- wgen/wgen_decl.cxx (revision 4043)
98 +++ wgen/wgen_decl.cxx (working copy)
100 WGEN_Generate_Temp_For_Initialized_Aggregate (gs_t init, char * name)
101 {
102 TY_IDX ty_idx = Get_TY(gs_tree_type(init));
103 - ST *temp = New_ST (CURRENT_SYMTAB);
104 - ST_Init (temp,
105 - Save_Str2 (name, ".init"),
106 - CLASS_VAR, SCLASS_PSTATIC, EXPORT_LOCAL,
107 - ty_idx );
108 + ST * temp = Make_Derived_Symbol(name,ty_idx);
109 AGGINIT agginit;
110 gs_code_t code = gs_tree_code(init);
111 if (code == GS_CONSTRUCTOR
113 DevWarn ("Static initialize %s(%s)\n",
114 ST_name(target_st),
Sclass_Name(ST_sclass(target_st)));
115 TY_IDX ty_idx = Get_TY(gs_tree_type(init));
116 - ST *temp = New_ST (CURRENT_SYMTAB);
117 - ST_Init (temp,
118 - Save_Str2 (ST_name(target_st), ".init"),
119 - CLASS_VAR, SCLASS_PSTATIC, EXPORT_LOCAL,
120 - ty_idx );
121 + ST *temp = Make_Derived_Symbol(ST_name(target_st),ty_idx);
122 // setup inito for target_st
123 Set_ST_is_initialized(temp);
124 agginit.Set_inito(New_INITO(temp));
125 Index: kg++fe/wfe_decl.cxx
126 ===================================================================
127 --- kg++fe/wfe_decl.cxx (revision 4043)
128 +++ kg++fe/wfe_decl.cxx (working copy)
130 WFE_Generate_Temp_For_Initialized_Aggregate (tree init, char * name)
131 {
132 TY_IDX ty_idx = Get_TY(TREE_TYPE(init));
133 - ST *temp = New_ST (CURRENT_SYMTAB);
134 - ST_Init (temp,
135 - Save_Str2 (name, ".init"),
136 - CLASS_VAR, SCLASS_PSTATIC, EXPORT_LOCAL,
137 - ty_idx );
138 + ST *temp = Make_Derived_Symbol(name,ty_idx);
139 if (TREE_CODE(init) == CONSTRUCTOR
140 && ! Use_Static_Init_For_Aggregate (temp, init))
141 {
143 DevWarn ("Static initialize %s(%s)\n",
144 ST_name(target_st),
Sclass_Name(ST_sclass(target_st)));
145 TY_IDX ty_idx = Get_TY(TREE_TYPE(init));
146 - ST *temp = New_ST (CURRENT_SYMTAB);
147 - ST_Init (temp,
148 - Save_Str2 (ST_name(target_st), ".init"),
149 - CLASS_VAR, SCLASS_PSTATIC, EXPORT_LOCAL,
150 - ty_idx );
151 + ST *temp = Make_Derived_Symbol(name,ty_idx);
152 // setup inito for target_st
153 Set_ST_is_initialized(temp);
154 aggregate_inito = New_INITO (temp);
Best wishes,
Liwei Zhang

----------------------------------------------------------------------------
--

Post by zhangliwei
INSIGHTS What's next for parallel hardware, programming and related areas?
Interviews and blogs by thought leaders keep you ahead of the curve.
http://goparallel.sourceforge.net
_______________________________________________
Open64-devel mailing list
https://lists.sourceforge.net/lists/listinfo/open64-devel

[zhangliwei]