This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix reassoc ICE (PR tree-optimization/69802)
- From: Jakub Jelinek <jakub at redhat dot com>
- To: Michael Matz <matz at suse dot de>
- Cc: Richard Biener <rguenther at suse dot de>, gcc-patches at gcc dot gnu dot org
- Date: Mon, 15 Feb 2016 22:41:49 +0100
- Subject: Re: [PATCH] Fix reassoc ICE (PR tree-optimization/69802)
- Authentication-results: sourceware.org; auth=none
- References: <20160215205434 dot GE3017 at tucnak dot redhat dot com> <alpine dot LSU dot 2 dot 20 dot 1602152222210 dot 20277 at wotan dot suse dot de>
- Reply-to: Jakub Jelinek <jakub at redhat dot com>
On Mon, Feb 15, 2016 at 10:27:16PM +0100, Michael Matz wrote:
> On Mon, 15 Feb 2016, Jakub Jelinek wrote:
>
> > + /* If op is default def SSA_NAME, there is no place to insert the
> > + new comparison. Give up, unless we can use OP itself as the
> > + range test. */
> > + if (op && SSA_NAME_IS_DEFAULT_DEF (op))
> > + {
> > + if (op == range->exp
> > + && ((TYPE_PRECISION (optype) == 1 && TYPE_UNSIGNED (optype))
> > + || TREE_CODE (optype) == BOOLEAN_TYPE)
> > + && (op == tem
> > + || (TREE_CODE (tem) == EQ_EXPR
> > + && TREE_OPERAND (tem, 0) == op
> > + && integer_onep (TREE_OPERAND (tem, 1))))
> > + && opcode != BIT_IOR_EXPR
> > + && (opcode != ERROR_MARK || oe->rank != BIT_IOR_EXPR))
>
> Perhaps just give up always, instead of this complicated (and hence
> fragile) hackery? Are you 100% sure you catched everything, are there
It is IMO not that fragile. The
op == range->exp is quite obvious condition, it could have been even assert
instead.
The second condition is what is used e.g. in init_range_test, i.e.
bool/unsigned :1 only. The third - op == tem is obviously good
transformation to tem = op, so the patch just makes sure we don't ICE
say in gsi_for_stmt (stmt); that is what we get for bool and is covered
in the testcase. The EQ_EXPR case is what we get for unsigned : 1 instead.
Perhaps it could be also op != 0 instead of just op == 1.
And lastly, the BIT_IOR_EXPR tests are to make sure we don't
if (opcode == BIT_IOR_EXPR
|| (opcode == ERROR_MARK && oe->rank == BIT_IOR_EXPR))
tem = invert_truthvalue_loc (loc, tem);
a few lines later. The reason to perform the check earlier is just
to avoid printing it in the dumpfile that we are changing something if we
give up instead.
Jakub