>> 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") ... > Did you mean this as a short-term or long-term solution? > (Mind, we already have a proposed short-term solution.) As a long term solution. Though not in that exact shape -- I wanted to have discussion on it and converge together to a real solution. >> (*) 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? > > Again, LEGITIMATE_CONSTANT_P is the wrong macro, it's for > checking constants which are appropriate as immediate operands > (to non-move insns), not for being at-all-legitimate. LEGITIMATE_CONSTANT_P is just what is used by general_operand. I'm proposing another use of *the predicate for mov's operand 1*, not of LEGITIMATE_CONSTANT_P. With the above questions, I was expressing my doubts on the doc for LEGITIMATE_CONSTANT_P in general. > Signalling that they are not legitimate means they can still be > handled by a move. That's why I used the predicate. >> 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. > To wit: a new bug would surface: you could here form something > that wasn't LEGITIMATE_CONSTANT_P but which was handled by a > move expander, and you'd force this into an insn which *isn't* a > move. N.B. the insn in PR36182 wasn't a move. Shouldn't the insn fail recognization, then? > (FWIW, I'll add a LEGITIMATE_CONSTANT_P to CRIS just to cover my > bases. It won't solve the basic problem, because that could > just cause that invalid CONST contents in PR37363 and PR36182 to > end up in a move insn instead.) I don't think so, because general_operand would pass the CONST to your LEGITIMATE_CONSTANT_P, and hence cause it to be rejected. Paolo