Re: Unexpected output constraints

2010-04-01 Thread Daniel Jacobowitz
On Fri, Apr 02, 2010 at 12:27:33AM +0100, Bernd Schmidt wrote:
> That doesn't work either, a matching constraint has to be an input.

i386 uses this, so I figure it's OK.

> Also, legitimize_tls_address is calling it with the same reg for both
> operands, which is going to lead to tears if they're both outputs.

Hrm.  Yeah, those really should be two pseudos.  I'll fix that.

-- 
Daniel Jacobowitz
CodeSourcery


Re: Unexpected output constraints

2010-04-01 Thread Bernd Schmidt
On 04/01/2010 11:08 PM, Daniel Jacobowitz wrote:

> (define_insn "tls_load_dot_plus_four"
>   [(set (match_operand:SI 0 "register_operand" "=l,r")
> (mem:SI (unspec:SI [(match_operand:SI 1 "register_operand" "l,r")
> (const_int 4)
> (match_operand 2 "" "")]
>UNSPEC_PIC_BASE)))
>(clobber (match_scratch:SI 3 "=&1,&1"))]

That doesn't work either, a matching constraint has to be an input.
Also, legitimize_tls_address is calling it with the same reg for both
operands, which is going to lead to tears if they're both outputs.  So,
maybe simply

 (define_insn "tls_load_dot_plus_four"
   [(set (match_operand:SI 0 "register_operand" "=l,r")
 (mem:SI (unspec:SI [(match_operand:SI 1 "register_operand" "0,0")
 (const_int 4)
 (match_operand 2 "" "")]
UNSPEC_PIC_BASE)))]

but on the other hand maybe it's best to find whoever wrote the pattern
and ask them what's going on.


Bernd


Re: Unexpected output constraints

2010-04-01 Thread Daniel Jacobowitz
On Fri, Apr 02, 2010 at 12:06:28AM +0100, Bernd Schmidt wrote:
> On 04/01/2010 10:54 PM, Daniel Jacobowitz wrote:
> > I'm debugging a Thumb-2 glibc build failure on trunk for
> > arm-none-linux-gnueabi.  I believe it's from Richard Earnshaw's
> > 2010-02-01 patch for TLS patterns, which includes this:
> > 
> > (define_insn "tls_load_dot_plus_four"
> >   [(set (match_operand:SI 0 "register_operand" "=l,r")
> > (mem:SI (unspec:SI [(match_operand:SI 1 "register_operand" "+l,r")
> > (const_int 4)
> > (match_operand 2 "" "")]
> >UNSPEC_PIC_BASE)))]
> > 
> > It's that "+" on the second operand.  It should just be "&", and I
> > think that will fix the failure (I'll test it shortly).
> 
> That can't be right, "&" only makes sense for outputs.

Yeah, suitable juggling in match_scratch was what I really needed:

(define_insn "tls_load_dot_plus_four"
  [(set (match_operand:SI 0 "register_operand" "=l,r")
(mem:SI (unspec:SI [(match_operand:SI 1 "register_operand" "l,r")
(const_int 4)
(match_operand 2 "" "")]
   UNSPEC_PIC_BASE)))
   (clobber (match_scratch:SI 3 "=&1,&1"))]

> > It seems to me that the problem is marking a register in the RHS of a
> > set as an output constraint.
> 
> Yes.  There was a similar bug for tls_load_dot_plus_eight recently.  In
> this case it seems that operand 1 is in fact both read and written by
> the pattern, so the pattern should be something like
> 
>   [(set (match_operand:SI 0 "register_operand" "=l,r")
> (mem:SI (unspec:SI [(match_dup 1)
> (const_int 4)
> (match_operand 2 "" "")]
>UNSPEC_PIC_BASE)))
>(clobber (match_operand:SI 1 "register_operand" "+&l,&r"))]

Is there a reason to prefer one of these forms over the other?
match_scratch / match_dup always make me ask that question :-)

-- 
Daniel Jacobowitz
CodeSourcery


Re: Unexpected output constraints

2010-04-01 Thread Bernd Schmidt
On 04/01/2010 10:54 PM, Daniel Jacobowitz wrote:
> I'm debugging a Thumb-2 glibc build failure on trunk for
> arm-none-linux-gnueabi.  I believe it's from Richard Earnshaw's
> 2010-02-01 patch for TLS patterns, which includes this:
> 
> (define_insn "tls_load_dot_plus_four"
>   [(set (match_operand:SI 0 "register_operand" "=l,r")
> (mem:SI (unspec:SI [(match_operand:SI 1 "register_operand" "+l,r")
> (const_int 4)
> (match_operand 2 "" "")]
>UNSPEC_PIC_BASE)))]
> 
> It's that "+" on the second operand.  It should just be "&", and I
> think that will fix the failure (I'll test it shortly).

That can't be right, "&" only makes sense for outputs.

[...]
> It seems to me that the problem is marking a register in the RHS of a
> set as an output constraint.

Yes.  There was a similar bug for tls_load_dot_plus_eight recently.  In
this case it seems that operand 1 is in fact both read and written by
the pattern, so the pattern should be something like

  [(set (match_operand:SI 0 "register_operand" "=l,r")
(mem:SI (unspec:SI [(match_dup 1)
(const_int 4)
(match_operand 2 "" "")]
   UNSPEC_PIC_BASE)))
   (clobber (match_operand:SI 1 "register_operand" "+&l,&r"))]

That's untested however.


Bernd


Unexpected output constraints

2010-04-01 Thread Daniel Jacobowitz
I'm debugging a Thumb-2 glibc build failure on trunk for
arm-none-linux-gnueabi.  I believe it's from Richard Earnshaw's
2010-02-01 patch for TLS patterns, which includes this:

(define_insn "tls_load_dot_plus_four"
  [(set (match_operand:SI 0 "register_operand" "=l,r")
(mem:SI (unspec:SI [(match_operand:SI 1 "register_operand" "+l,r")
(const_int 4)
(match_operand 2 "" "")]
   UNSPEC_PIC_BASE)))]

It's that "+" on the second operand.  It should just be "&", and I
think that will fix the failure (I'll test it shortly).  But I'm
looking at what reload does before the failure.

The failing insn is 104, in this sequence:

(insn 103 79 175 2 delta/in7c.i:50 (set (reg/f:SI 217)
(const:SI (unspec:SI [
(symbol_ref:SI ("*.LANCHOR0") [flags 0x1a2])
(const_int 3 [0x3])
(const (unspec:SI [
(const_int 3 [0x3])
] 21))
(const_int 4 [0x4])
] 20))) 658 {*thumb2_movsi_insn} (nil))

...

(insn 104 102 107 11 delta/in7c.i:50 (set (reg:SI 203)
(mem:SI (unspec:SI [
(reg/f:SI 217)
(const_int 4 [0x4])
(const_int 3 [0x3])
] 4) [0 S4 A32])) 659 {tls_load_dot_plus_four} (nil))

For #104, reg_equiv_constant[217] is set to the (const) from #103.
And because r217 is marked as in-out from tls_load_dot_plus_four,
the reload is RELOAD_READ_WRITE.  If the operand is an output, then
it can't be correct to replace it with a constant.  We ICE
trying to emit (set (const) (reg)).

It seems to me that the problem is marking a register in the RHS of a
set as an output constraint.  The reg becomes function_invariant_p and
chaos ensues.

Is this right?  If so, is there somewhere that should assert if an
operand's constraint is marked as an output, but not somewhere that
the RTL allows modification of the operand?

-- 
Daniel Jacobowitz
CodeSourcery