Re: [IA-64] Fix PR target/48496
> Is the standard condition for define_memory_constraint here > /* Likewise if the address will be reloaded because >reg_equiv_address is nonzero. For reg_equiv_mem >we have to check. */ > else if (REG_P (operand) > && REGNO (operand) >= > FIRST_PSEUDO_REGISTER && reg_renumber[REGNO (operand)] < 0 && > ((reg_equiv_mem (REGNO (operand)) != 0 && EXTRA_CONSTRAINT_STR > (reg_equiv_mem (REGNO (operand)), c, p)) > > || (reg_equiv_address (REGNO > || (operand)) != 0))) > > win = 1; > If so, shouldn't you check those conditions as well, or at least something > similar? Not sure if reg_equiv_address needs to be allowed there, and > guess reg_equiv_mem should satisfy the Q constraint, i.e. !MEM_VOLATILE_P > memory_operand. Accepting any pseudo there sounds too risky to me... You're right, and modifying a constraint to silence a bogus error is probably too dangerous in any case. And there may be other affected architectures. So patch withdrawn. The best fix is very likely in reload1.c in the end. -- Eric Botcazou
Re: [IA-64] Fix PR target/48496
On Wed, Apr 11, 2012 at 11:01:40PM +0200, Eric Botcazou wrote: > 2012-04-11 Eric Botcazou > > PR target/48496 > * config/ia64/constraints.md (Q): Only accept non-volatile MEMs and > also pseudo-registers during reload. > > > 2012-04-11 Eric Botcazou > > * gcc.target/ia64/pr48496.c: New test. > * gcc.target/ia64/pr52657.c: Likewise. Is the standard condition for define_memory_constraint here /* Likewise if the address will be reloaded because reg_equiv_address is nonzero. For reg_equiv_mem we have to check. */ else if (REG_P (operand) && REGNO (operand) >= FIRST_PSEUDO_REGISTER && reg_renumber[REGNO (operand)] < 0 && ((reg_equiv_mem (REGNO (operand)) != 0 && EXTRA_CONSTRAINT_STR (reg_equiv_mem (REGNO (operand)), c, p)) || (reg_equiv_address (REGNO (operand)) != 0))) win = 1; If so, shouldn't you check those conditions as well, or at least something similar? Not sure if reg_equiv_address needs to be allowed there, and guess reg_equiv_mem should satisfy the Q constraint, i.e. !MEM_VOLATILE_P memory_operand. Accepting any pseudo there sounds too risky to me... > Index: config/ia64/constraints.md > === > --- config/ia64/constraints.md(revision 186272) > +++ config/ia64/constraints.md(working copy) > @@ -111,11 +111,16 @@ (define_constraint "H" > > ;; Note that while this accepts mem, it only accepts non-volatile mem, > ;; and so cannot be "fixed" by adjusting the address. Thus it cannot > -;; and does not use define_memory_constraint. > +;; and does not use define_memory_constraint. But it needs to accept > +;; pseudo-registers during reload like a define_memory_constraint. > (define_constraint "Q" >"Non-volatile memory for FP_REG loads/stores" > - (and (match_operand 0 "memory_operand") > - (match_test "!MEM_VOLATILE_P (op)"))) > + (ior (and (match_code "mem") > + (match_test "!MEM_VOLATILE_P (op)") > + (match_operand 0 "memory_operand")) > + (and (match_code "reg") > + (match_test "!HARD_REGISTER_P (op)") > + (match_test "reload_in_progress" > > (define_constraint "R" >"1..4 for shladd arguments" Jakub
Re: [IA-64] Fix PR target/48496
On Wed, Apr 11, 2012 at 11:01 PM, Eric Botcazou wrote: > Hi, > > this is the spurious error on asm statements of the form: > > error: 'asm' operand requires impossible reload > > present for IA-64 on mainline and 4.7 branch. As diagnosed by Ulrich, the > code > responsible for the error implicitly assumes that constraints accepting memory > operands also accept pseudo-registers during reload. That isn't true for the > Q constraint of IA-64. The patch also fixes the MEM_VOLATILE_P access not > properly guarded by a MEM_P test in the expression. > > Bootstrapped/regtested on IA-64/Linux, OK for mainline and 4.7 branch? How ugly ;) Can we have a generic helper for the pseudo-created-by-reload test on trunk please? Ok. Thanks, Richard. > > 2012-04-11 Eric Botcazou > > PR target/48496 > * config/ia64/constraints.md (Q): Only accept non-volatile MEMs and > also pseudo-registers during reload. > > > 2012-04-11 Eric Botcazou > > * gcc.target/ia64/pr48496.c: New test. > * gcc.target/ia64/pr52657.c: Likewise. > > > -- > Eric Botcazou
