Paolo Bonzini <[EMAIL PROTECTED]> writes: >>> 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. >> >> I think you're mixing up CRIS and rs6000, the latter which >> generated something it had to handle but which was munged, >> PR36090. CRIS is mainstream in that sense. (You'd have to get >> buy-in from David Edelsohn on a LEGITIMATE_CONSTANT_P definition >> in rs6000 if PR36090 resurfaces.) > > This is from CRIS: > > (define_expand "movsi" > [(set > (match_operand:SI 0 "nonimmediate_operand" "") > (match_operand:SI 1 "cris_general_operand_or_symbol" ""))] > ... > > (define_special_predicate "cris_general_operand_or_symbol" > (ior (match_operand 0 "general_operand") > (and (match_code "const, symbol_ref, label_ref") > ...
As H-P says, the predicates on move expanders are generally ignored. emit_move_insn & subroutines deliberately don't check them. (Our usual method of making things match predicates is to force them into a register, but that could easily lead to infinite recursion if applied to moves.) FWIW, in MIPS, I have a port policy of not adding predicates to move expanders. I think adding them just creates confusion, because readers will think they actually mean something. >>> 2) it makes clear how to fix bugs -- you restrict >>> LEGITIMATE_CONSTANT_P/LEGITIMATE_PIC_OPERAND_P or add a move expander. >> >> Contradicting current use, where anything that's found in a >> non-LEGITIMATE_CONSTANT_P/LEGITIMATE_PIC_OPERAND_P must be >> handled by a move expander! > > Not necessarily; anything that's found in a non-legitimate constant must > be handled by force_reg, and force_reg also tries using force_operand if > what it gets is not a general_operand. But maybe it's necessary to add a > > if (GET_CODE (value) == CONST) > value = XEXP (value, 0); > > in force_operand. As you say, force_operand currently does nothing with constants. My understanding is that that really is by design (in the loosest possible sense of the word). As H-P says, it's then the move expander's responsibility to handle the thing. I know it's not the cleanest interface, but many ports are written to follow this behaviour. TBH, as I said in comment #11 in PR36182, I think rs6000 is being overly cute with the representation of a TOC entry as: (const (minus foo (symbol_ref "*.LCTOC1"))). It might be accurate, but (symbol_ref "*.LCTOC1") has no real meaning to GCC. And as we can see from these bugs, the extra symbol_ref just tends to get in the way. Other ports would generally use an unspec here. (E.g. most ports use an unspec for GOT-relative constants, even though: (const (minus foo (symbol_ref "_GLOBAL_OFFSET_TABLE_"))) would in some cases be accurate.) I think using an unspec in rs6000 would solve some of the port-specific issues. In particular, I don't think 36090 would have happened with an unspec representation. Richard
