> if plus_constant _knows_ that something can be wrapped in a CONST, > simplify_binary_operation should have given us the CONST to begin with. > Also, the only cases that plus_constant can handle are CONST, > SYMBOL_REF and LABEL_REF, all of which satisfy CONSTANT_P. > So the new form ought to be dead on two counts.
Yes, and in the other case too: ops[i - 1].op = gen_rtx_MINUS (mode, ops[i - 1].op, ops[i].op); ops[i - 1].op = plus_constant (ops[i - 1].op, 0); plus_constant won't understand the MINUS, and won't generate a CONST. Still, having a new target hook for this seems overkill. For example, since ports do have to deal with complicated constants when they expand moves, and since some of them already look inside CONSTs in their LEGITIMATE_CONSTANT_P, another possibility to throw in the air is something like (better names welcome...) rtx avoid_terrible_constants (rtx x) { if (!CONSTANT_P (x)) x = gen_rtx_CONST (x); /* If the target's move expanders will take care of it, it must not be that bad. */ icode = optab_handler (mov_optab, GET_MODE (x))->insn_code; if (*insn_data[icode].operand[1].predicate (x, GET_MODE (x))) return x; return NULL; } In case of cris, the predicate goes into general_operand, which does if (CONSTANT_P (op)) return ((GET_MODE (op) == VOIDmode || GET_MODE (op) == mode || mode == VOIDmode) && (! flag_pic || LEGITIMATE_PIC_OPERAND_P (op)) && LEGITIMATE_CONSTANT_P (op)); H-P can check for the problematic case inside his LEGITIMATE_CONSTANT_P (*), or add a move expander for it. (*) but then does this mean the documentation for L_C_P is obsolete, and returning 1 is not necessarily a good thing to do for targets with sections? Maybe there is a better definition that can be the default? Anyway, at least how to use this function is pretty obvious: tem_rhs = GET_CODE (rhs) == CONST ? XEXP (rhs, 0) : rhs; tem = simplify_binary_operation (ncode, mode, tem_lhs, tem_rhs); - if (tem && !CONSTANT_P (tem)) - tem = gen_rtx_CONST (GET_MODE (tem), tem); + if (tem) + tem = avoid_terrible_constants (tem); } else tem = simplify_binary_operation (ncode, mode, lhs, rhs); ... && CONSTANT_P (ops[i].op) && GET_CODE (ops[i].op) == GET_CODE (ops[i - 1].op)) { - ops[i - 1].op = gen_rtx_MINUS (mode, ops[i - 1].op, ops[i].op); - ops[i - 1].op = gen_rtx_CONST (mode, ops[i - 1].op); - if (i < n_ops - 1) - ops[i] = ops[i + 1]; - n_ops--; + rtx x; + x = gen_rtx_MINUS (mode, ops[i - 1].op, ops[i].op); + x = avoid_terrible_constants (x); + if (x) + { + ops[i - 1].op = x; + if (i < n_ops - 1) + ops[i] = ops[i + 1]; + n_ops--; + } } if (n_ops > 1 I'm absolutely unsure that this is the way to go; but it has two advantages: 1) not leaking really bad constants outside simplify-rtx.c; 2) it makes clear how to fix bugs -- you restrict LEGITIMATE_CONSTANT_P/LEGITIMATE_PIC_OPERAND_P or add a move expander. Paolo