| Message ID | 54E99B0E.2080109@sunrus.com.cn |
|---|---|
| State | New |
| Headers | show |
On 02/22/15 02:02, Chen Gang S wrote: > It is for Bug65117, after this fix, ".i" file can be passed compiling. > > - 'this_alternative_win' is not initialized before use it: for the > first looping 0, it initializes 'this_alternative_win[0]', but > 'did_match' may use 'this_alternative_win[2]'. > > - 'this_alternative' may be not initialized before using: it > initializes 'this_alternative[i]', but may use 'this_alternative[m]' > (m > i). > > - After reading through the code, arrays 'this_alternative_match_win', > 'this_alternative_offmemok', and 'this_alternative_earlyclobber' may > be not initialized either, so initialize them too. > > This issue is found by cross compiling xtensa Linux kernel with the > latest gcc5. And after this patch, it can cross compile xtensa Linux > kernel with allmodconfig, successfully. > > > 2015-02-22 Chen Gang <gang.chen.5i5j@gmail.com> > > * reload.c (find_reloads): Initialize several arrays before use > them. From the documentation for matching constraints: Moreover, the digit must be a smaller number than the number of the operand that uses it in the constraint. If we look at the zero_cost_loop_{start,end} patterns we have: (if_then_else (ne (match_operand:SI 0 "register_operand" "2") and (if_then_else (ne (match_operand:SI 0 "nonimmediate_operand" "2,2") Similarly for the loop_end pattern. Which violate the rule for matching constraints. I'm confident that if the xtensa's patterns were fixed to abide by the rules for matching constraints the problem in reload would not occur. jeff
On Mon, Feb 23, 2015 at 11:26 PM, Jeff Law wrote: > On 02/22/15 02:02, Chen Gang S wrote: >> >> It is for Bug65117, after this fix, ".i" file can be passed compiling. >> >> - 'this_alternative_win' is not initialized before use it: for the >> first looping 0, it initializes 'this_alternative_win[0]', but >> 'did_match' may use 'this_alternative_win[2]'. >> >> - 'this_alternative' may be not initialized before using: it >> initializes 'this_alternative[i]', but may use 'this_alternative[m]' >> (m > i). >> >> - After reading through the code, arrays 'this_alternative_match_win', >> 'this_alternative_offmemok', and 'this_alternative_earlyclobber' may >> be not initialized either, so initialize them too. >> >> This issue is found by cross compiling xtensa Linux kernel with the >> latest gcc5. And after this patch, it can cross compile xtensa Linux >> kernel with allmodconfig, successfully. >> >> >> 2015-02-22 Chen Gang <gang.chen.5i5j@gmail.com> >> >> * reload.c (find_reloads): Initialize several arrays before use >> them. > > > From the documentation for matching constraints: > > Moreover, the digit must be a > smaller number than the number of the operand that uses it in the > constraint. > > > If we look at the zero_cost_loop_{start,end} patterns we have: > > (if_then_else (ne (match_operand:SI 0 "register_operand" "2") > > and > > > (if_then_else (ne (match_operand:SI 0 "nonimmediate_operand" "2,2") > > Similarly for the loop_end pattern. > > > Which violate the rule for matching constraints. ...and should never have worked at all... > I'm confident that if the xtensa's patterns were fixed to abide by the rules > for matching constraints the problem in reload would not occur. Chen, perhaps a warming can be implemented for this in genrecog? Ciao! Steven
On 02/23/15 16:09, Steven Bosscher wrote: >> >> >> Which violate the rule for matching constraints. > > ...and should never have worked at all... Yup. It's only been fairly recently that we started statically checking MD files in any significant way -- we've still got a long way to go I'm sure. > > >> I'm confident that if the xtensa's patterns were fixed to abide by the rules >> for matching constraints the problem in reload would not occur. > > > Chen, perhaps a warming can be implemented for this in genrecog? That would certainly be a welcome addition! jeff
On 2/24/15 07:14, Jeff Law wrote: > On 02/23/15 16:09, Steven Bosscher wrote: >>> >>> >>> Which violate the rule for matching constraints. >> >> ...and should never have worked at all... > Yup. It's only been fairly recently that we started statically checking MD files in any significant way -- we've still got a long way to go I'm sure. > OK, thanks. I shall try to finish within this month (although I am not quite sure whether I can finish on time). >> >> >>> I'm confident that if the xtensa's patterns were fixed to abide by the rules >>> for matching constraints the problem in reload would not occur. >> >> >> Chen, perhaps a warming can be implemented for this in genrecog? > That would certainly be a welcome addition! > OK, thanks. May I firstly finish it before fixing xtensa pattern? I guess it is more easier than fixing xtensa patten. (I guess 'warming' is 'warning'.) By the way, can this patch "initialize several arrays before use them in find_reloads()" will cause correctness issue? (I guess not, it will skip optimization, but not cause correctness issue, and continue compiling). Thanks.
diff --git a/gcc/reload.c b/gcc/reload.c index 70b86a9..7e83c10 100644 --- a/gcc/reload.c +++ b/gcc/reload.c @@ -2666,10 +2666,10 @@ find_reloads (rtx_insn *insn, int replace, int ind_levels, int live_known, int no_input_reloads = 0, no_output_reloads = 0; int n_alternatives; reg_class_t this_alternative[MAX_RECOG_OPERANDS]; - char this_alternative_match_win[MAX_RECOG_OPERANDS]; - char this_alternative_win[MAX_RECOG_OPERANDS]; - char this_alternative_offmemok[MAX_RECOG_OPERANDS]; - char this_alternative_earlyclobber[MAX_RECOG_OPERANDS]; + char this_alternative_match_win[MAX_RECOG_OPERANDS] = {0}; + char this_alternative_win[MAX_RECOG_OPERANDS] = {0}; + char this_alternative_offmemok[MAX_RECOG_OPERANDS] = {0}; + char this_alternative_earlyclobber[MAX_RECOG_OPERANDS] = {0}; int this_alternative_matches[MAX_RECOG_OPERANDS]; reg_class_t goal_alternative[MAX_RECOG_OPERANDS]; int this_alternative_number; @@ -2692,6 +2692,8 @@ find_reloads (rtx_insn *insn, int replace, int ind_levels, int live_known, machine_mode operand_mode[MAX_RECOG_OPERANDS]; int retval = 0; + for (i = 0; i < MAX_RECOG_OPERANDS; i++) + this_alternative[i] = NO_REGS; this_insn = insn; n_reloads = 0; n_replacements = 0;
It is for Bug65117, after this fix, ".i" file can be passed compiling. - 'this_alternative_win' is not initialized before use it: for the first looping 0, it initializes 'this_alternative_win[0]', but 'did_match' may use 'this_alternative_win[2]'. - 'this_alternative' may be not initialized before using: it initializes 'this_alternative[i]', but may use 'this_alternative[m]' (m > i). - After reading through the code, arrays 'this_alternative_match_win', 'this_alternative_offmemok', and 'this_alternative_earlyclobber' may be not initialized either, so initialize them too. This issue is found by cross compiling xtensa Linux kernel with the latest gcc5. And after this patch, it can cross compile xtensa Linux kernel with allmodconfig, successfully. 2015-02-22 Chen Gang <gang.chen.5i5j@gmail.com> * reload.c (find_reloads): Initialize several arrays before use them. --- gcc/reload.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)