Jakub Jelinek <ja...@redhat.com> writes: > On Tue, Apr 15, 2014 at 09:53:16PM +0100, Richard Sandiford wrote: >> As Robert pointed out here: >> >> http://gcc.gnu.org/ml/gcc-patches/2014-04/msg00416.html >> >> we're a bit too eager when folding stuff into an 'X' constraint. >> The value at expand time is sensible, but after that asm_operand_ok >> allows arbitrary rtx expressions, including any number of registers >> as well as MEMs with unchecked addresses. >> >> This is a target-independent problem, as shown by the testcase below. >> Reload would give bogus "impossible constraint in asm" errors >> while LRA ICEs. >> >> Tested on x86_64-linux-gnu. OK to install? > > But then what will be "X" good for compared to "gin" or similar? > X constraint is meant for operands that aren't really needed, trying to > print is a user error. > > I guess the documentation agrees with this: > > 'X' > Any operand whatsoever is allowed, even if it does not satisfy > 'general_operand'. This is normally used in the constraint of a > 'match_scratch' when certain alternatives will not actually require > a scratch register. > > So I think we should just error out if somebody tries to print something > that satisfies X constraint.
That's the internal documentation though, whereas here we're checking asm uses. The documentation for asms just says "Any operand whatsoever is allowed." It doesn't say anything about it being unprintable. I just added the printing side for completeness though. It wasn't the point of the patch. Like I say, the point is that LRA ICEs (on x86_64) because it can't reload the operands. Reload couldn't reload them either but raised "impossible constraint in asm" errors instead of internal errors. (IMO those errors were also invalid though. If "X" allows "any operand whatsoever", how can the operands in the testcase be invalid?) "X" was defined against reload, which always reloaded MEM addresses to follow the appropriate base and index register classes. This was done as a first pass before matching against the constraints: /* Examine each operand that is a memory reference or memory address and reload parts of the addresses into index registers. Also here any references to pseudo regs that didn't get hard regs but are equivalent to constants get replaced in the insn itself with those constants. Nobody will ever see them again. Finally, set up the preferred classes of each operand. */ for (i = 0; i < noperands; i++) { ... else if (code == MEM) { address_reloaded[i] = find_reloads_address (GET_MODE (recog_data.operand[i]), recog_data.operand_loc[i], XEXP (recog_data.operand[i], 0), &XEXP (recog_data.operand[i], 0), i, address_type[i], ind_levels, insn); recog_data.operand[i] = *recog_data.operand_loc[i]; substed_operand[i] = recog_data.operand[i]; So I don't think it has ever been the case that "X" allowed MEMs with arbitrary expressions as the address. IMO the point of "X" (as implied the doc you quoted) is that it allows (scratch) operands to be kept as (scratch)s in cases where no scratch register is needed. Thanks, Richard