Message ID | 73fbd025-06d0-0641-00e2-a47bd8879e6a@codesourcery.com |
---|---|
State | New |
Headers | show |
Series | Fix ICE in re-simplification of VEC_COND_EXPR (was: Re: [PATCH][amdgcn] Fix ICE in re-simplification of VEC_COND_EXPR) | expand |
On Fri, Nov 29, 2019 at 02:38:34PM +0100, Harwath, Frederik wrote: > 2019-11-29 Frederik Harwath <frederik@codesourcery.com> > > gcc/ > * gimple-match-head.c (maybe_resimplify_conditional_op): use s/use/Use/ > generic_expr_could_trap_p to check if the condition of COND_EXPR or > VEC_COND_EXPR can trap. > --- > gcc/gimple-match-head.c | 18 +++++++++++++++--- > 1 file changed, 15 insertions(+), 3 deletions(-) > > diff --git a/gcc/gimple-match-head.c b/gcc/gimple-match-head.c > index 2996bade301..c763a80a6d1 100644 > --- a/gcc/gimple-match-head.c > +++ b/gcc/gimple-match-head.c > @@ -144,9 +144,21 @@ maybe_resimplify_conditional_op (gimple_seq *seq, gimple_match_op *res_op, > /* Likewise if the operation would not trap. */ > bool honor_trapv = (INTEGRAL_TYPE_P (res_op->type) > && TYPE_OVERFLOW_TRAPS (res_op->type)); > - if (!operation_could_trap_p ((tree_code) res_op->code, > - FLOAT_TYPE_P (res_op->type), > - honor_trapv, res_op->op_or_null (1))) > + tree_code op_code = (tree_code) res_op->code; > + bool op_could_trap; > + > + /* COND_EXPR and VEC_COND_EXPR will trap if, and only if, the condition > + traps and hence we have to check this. For all other operations, we s/. /. / > + don't need to consider the operands. */ Likewise. Jakub
Hi Jakub, On 29.11.19 14:41, Jakub Jelinek wrote: > s/use/Use/ > > [...] > > s/. /. / Right, thanks. Does that look ok for inclusion in trunk now? Best regards, Frederik 2019-11-29 Frederik Harwath <frederik@codesourcery.com> gcc/ * gimple-match-head.c (maybe_resimplify_conditional_op): Use generic_expr_could_trap_p to check if the condition of COND_EXPR or VEC_COND_EXPR can trap. --- gcc/gimple-match-head.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/gcc/gimple-match-head.c b/gcc/gimple-match-head.c index 2996bade301..9010f11621e 100644 --- a/gcc/gimple-match-head.c +++ b/gcc/gimple-match-head.c @@ -144,9 +144,21 @@ maybe_resimplify_conditional_op (gimple_seq *seq, gimple_match_op *res_op, /* Likewise if the operation would not trap. */ bool honor_trapv = (INTEGRAL_TYPE_P (res_op->type) && TYPE_OVERFLOW_TRAPS (res_op->type)); - if (!operation_could_trap_p ((tree_code) res_op->code, - FLOAT_TYPE_P (res_op->type), - honor_trapv, res_op->op_or_null (1))) + tree_code op_code = (tree_code) res_op->code; + bool op_could_trap; + + /* COND_EXPR and VEC_COND_EXPR will trap if, and only if, the condition + traps and hence we have to check this. For all other operations, we + don't need to consider the operands. */ + if (op_code == COND_EXPR || op_code == VEC_COND_EXPR) + op_could_trap = generic_expr_could_trap_p (res_op->ops[0]); + else + op_could_trap = operation_could_trap_p ((tree_code) res_op->code, + FLOAT_TYPE_P (res_op->type), + honor_trapv, + res_op->op_or_null (1)); + + if (!op_could_trap) { res_op->cond.cond = NULL_TREE; return false;
"Harwath, Frederik" <frederik@codesourcery.com> writes: > Hi Jakub, > > On 29.11.19 14:41, Jakub Jelinek wrote: > >> s/use/Use/ >> >> [...] >> >> s/. /. / > > Right, thanks. Does that look ok for inclusion in trunk now? > > Best regards, > Frederik > > > 2019-11-29 Frederik Harwath <frederik@codesourcery.com> > > gcc/ > * gimple-match-head.c (maybe_resimplify_conditional_op): Use > generic_expr_could_trap_p to check if the condition of COND_EXPR or > VEC_COND_EXPR can trap. Thanks for doing this, looks good to me FWIW. I was seeing the same failure for SVE but hadn't found time to look at it. Richard > --- > gcc/gimple-match-head.c | 18 +++++++++++++++--- > 1 file changed, 15 insertions(+), 3 deletions(-) > > diff --git a/gcc/gimple-match-head.c b/gcc/gimple-match-head.c > index 2996bade301..9010f11621e 100644 > --- a/gcc/gimple-match-head.c > +++ b/gcc/gimple-match-head.c > @@ -144,9 +144,21 @@ maybe_resimplify_conditional_op (gimple_seq *seq, gimple_match_op *res_op, > /* Likewise if the operation would not trap. */ > bool honor_trapv = (INTEGRAL_TYPE_P (res_op->type) > && TYPE_OVERFLOW_TRAPS (res_op->type)); > - if (!operation_could_trap_p ((tree_code) res_op->code, > - FLOAT_TYPE_P (res_op->type), > - honor_trapv, res_op->op_or_null (1))) > + tree_code op_code = (tree_code) res_op->code; > + bool op_could_trap; > + > + /* COND_EXPR and VEC_COND_EXPR will trap if, and only if, the condition > + traps and hence we have to check this. For all other operations, we > + don't need to consider the operands. */ > + if (op_code == COND_EXPR || op_code == VEC_COND_EXPR) > + op_could_trap = generic_expr_could_trap_p (res_op->ops[0]); > + else > + op_could_trap = operation_could_trap_p ((tree_code) res_op->code, > + FLOAT_TYPE_P (res_op->type), > + honor_trapv, > + res_op->op_or_null (1)); > + > + if (!op_could_trap) > { > res_op->cond.cond = NULL_TREE; > return false;
On 29.11.19 15:46, Richard Sandiford wrote: > Thanks for doing this, looks good to me FWIW. I was seeing the same > failure for SVE but hadn't found time to look at it. Thank you all for the review. Committed as r278853. Frederik
diff --git a/gcc/gimple-match-head.c b/gcc/gimple-match-head.c index 2996bade301..c763a80a6d1 100644 --- a/gcc/gimple-match-head.c +++ b/gcc/gimple-match-head.c @@ -144,9 +144,21 @@ maybe_resimplify_conditional_op (gimple_seq *seq, gimple_match_op *res_op, /* Likewise if the operation would not trap. */ bool honor_trapv = (INTEGRAL_TYPE_P (res_op->type) && TYPE_OVERFLOW_TRAPS (res_op->type)); - if (!operation_could_trap_p ((tree_code) res_op->code, - FLOAT_TYPE_P (res_op->type), - honor_trapv, res_op->op_or_null (1))) + tree_code op_code = (tree_code) res_op->code; + bool op_could_trap; + + /* COND_EXPR and VEC_COND_EXPR will trap if, and only if, the condition + traps and hence we have to check this. For all other operations, we + don't need to consider the operands. */ + if (op_code == COND_EXPR || op_code == VEC_COND_EXPR) + op_could_trap = generic_expr_could_trap_p (res_op->ops[0]); + else + op_could_trap = operation_could_trap_p ((tree_code) res_op->code, + FLOAT_TYPE_P (res_op->type), + honor_trapv, + res_op->op_or_null (1)); + + if (!op_could_trap) { res_op->cond.cond = NULL_TREE; return false;