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