Constraints and reload

Constraints and reload

[Jean Christophe Beyler]

Dear all,

I was working on simplifying the movdi in my port and I had put :

  [(set (match_operand:DI 0 "nonimmediate_operand" "=r,r,r,o")
        (match_operand:DI 1 "general_operand"      "r,i,o,r"))]

However, due to constraints on the offset that is allowed, I:

- Added the check of the offset in the expand
- Added the check of the offset in the definition of the instruction
- Changed the constraints to:

  [(set (match_operand:DI 0 "nonimmediate_operand" "=r,r,r,R")
        (match_operand:DI 1 "general_operand"      "r,i,R,r"))]

Where R checks if the operand is a memory operand and if the offset is correct.

Once I did that, in one case of my regression tests, I received the error:
asm operand requires impossible reload

However, if I generate what I get in the first version, all the moves
seem to be correct anyway.

My theory is that reload needs the o EVEN IF what it will be trying to
do is illegal and it will figure that out and finally generate the
right thing afterwards.
Thus, why the o is necessary and not the R.

Is this correct?

Thanks,
Jean Christophe Beyler

Re: Constraints and reload

[Richard Henderson]

On 09/14/2009 12:18 PM, Jean Christophe Beyler wrote:
>    [(set (match_operand:DI 0 "nonimmediate_operand" "=r,r,r,R")
>          (match_operand:DI 1 "general_operand"      "r,i,R,r"))]
>
> Where R checks if the operand is a memory operand and if the offset is correct.

Did you use define_memory_constraint for R, or just define_constraint?
If the former, it's a bug in reload; if the later, that's the problem.


r~

Re: Constraints and reload

[Jean Christophe Beyler]

Sorry to bring this back to the conversation. Is there any reason why
this would not work with floating-point constraints ?

My "R" constraint is defined as:

(define_memory_constraint "R"
  "R is for memory references which take 1 word for the instruction"
  (and (match_code "mem")
       (match_test "simple_memory_operand (op)")))

I have :
(define_insn "movdf_internal"
  [(set (match_operand:DF 0 "nonimmediate_operand" "=r,r,r,R")
        (match_operand:DF 1 "general_operand"      " r,F,R,r"))]
  "check_move (operands[0], operands[1])"
   "@
    mov\\t%0,%1
    lid\\t%0,%1
    ldd\\t%0,%1
    std\\t%1,%0"
  [(set_attr "type"     "move,arith,load,store")
   (set_attr "mode"     "DF,DF,DF,DF")
   (set_attr "length"   "1,1,1,1")])

However, on a relatively complex bit of code, I get :
error: unrecognizable insn:
(set (mem/s:DF (const:DI (plus:DI (symbol_ref:DI ("st") <var_decl
0x2a957d7aa0 st>)
                    (const_int 48 [0x30]))) [14 st+48 S8 A64])
        (reg:DF 10 r10)) -1 (nil))

(My architecture cannot allow const as addresses to the store, it must
go through a register).

Basically, I have the same expand code and the same check_move
functions for the integer modes since they check the same things: is
it a memory, does it have an acceptable offset.

Check_move and simple_memory return false to the destination operand
of this set and the expand phase put "st" in a register.

But duing the greg pass, it shows:

Reloads for insn # 140
Reload 0: reload_out (DF) = (mem/s:DF (const:DI (plus:DI
(symbol_ref:DI ("st") <var_decl 0x2a957d7aa0 st>)
                                                            (const_int
48 [0x30]))) [14 st+48 S8 A64])
        GR_REGS, RELOAD_FOR_OUTPUT (opnum = 0)
        reload_out_reg: (mem/s:DF (const:DI (plus:DI (symbol_ref:DI
("st") <var_decl 0x2a957d7aa0 st>)
                                                            (const_int
48 [0x30]))) [14 st+48 S8 A64])
        reload_reg_rtx: (reg:DF 10 r10)

It is theorically possible that my code fails for integer modes but
that my code example doesn't force the compiler in the same route and
thus I don't see a fail. But I don't really think so.

Any ideas ?

Thanks a lot,
Jc

On Mon, Sep 14, 2009 at 2:16 PM, Jean Christophe Beyler
<jean.christophe.beyler@gmail.com> wrote:
> That seems to have fixed it, I just reverted back to what I had done initially.
>
> Thanks a lot,
> Jc
>
> On Mon, Sep 14, 2009 at 1:46 PM, Richard Henderson <rth@redhat.com> wrote:
>> On 09/14/2009 12:18 PM, Jean Christophe Beyler wrote:
>>>
>>>   [(set (match_operand:DI 0 "nonimmediate_operand" "=r,r,r,R")
>>>         (match_operand:DI 1 "general_operand"      "r,i,R,r"))]
>>>
>>> Where R checks if the operand is a memory operand and if the offset is
>>> correct.
>>
>> Did you use define_memory_constraint for R, or just define_constraint?
>> If the former, it's a bug in reload; if the later, that's the problem.
>>
>>
>> r~
>>
>

Re: Constraints and reload

[Richard Henderson]

On 09/16/2009 03:00 PM, Jean Christophe Beyler wrote:
> Sorry to bring this back to the conversation. Is there any reason why
> this would not work with floating-point constraints ?

Not that I can think of.  Did you provide all of the secondary reload
stuff that you need?  Probably not.

> (define_memory_constraint "R"
>    "R is for memory references which take 1 word for the instruction"
>    (and (match_code "mem")
>         (match_test "simple_memory_operand (op)")))
>
> I have :
> (define_insn "movdf_internal"
>    [(set (match_operand:DF 0 "nonimmediate_operand" "=r,r,r,R")
>          (match_operand:DF 1 "general_operand"      " r,F,R,r"))]
>    "check_move (operands[0], operands[1])"
>     "@
>      mov\\t%0,%1
>      lid\\t%0,%1
>      ldd\\t%0,%1
>      std\\t%1,%0"
>    [(set_attr "type"     "move,arith,load,store")
>     (set_attr "mode"     "DF,DF,DF,DF")
>     (set_attr "length"   "1,1,1,1")])
>
> However, on a relatively complex bit of code, I get :
> error: unrecognizable insn:
> (set (mem/s:DF (const:DI (plus:DI (symbol_ref:DI ("st")<var_decl
> 0x2a957d7aa0 st>)
>                      (const_int 48 [0x30]))) [14 st+48 S8 A64])
>          (reg:DF 10 r10)) -1 (nil))
>
> (My architecture cannot allow const as addresses to the store, it must
> go through a register).

You'll do much better by rejecting these addresses earlier.
Probably by some combination of custom predicates that allow
only simple_memory_operand + registers + constants, or by
simply defining this kind of memory address illegal for DFmode.



r~

Re: Constraints and reload

[Jean Christophe Beyler]

> Not that I can think of.  Did you provide all of the secondary reload
> stuff that you need?  Probably not.

Probably not, I've been working at cleaning up what was done before.
What exactly is needed to be done to define the secondary reload stuff
?

> You'll do much better by rejecting these addresses earlier.
> Probably by some combination of custom predicates that allow
> only simple_memory_operand + registers + constants, or by
> simply defining this kind of memory address illegal for DFmode.

That is what I thought I had done:

- DImode: accepts move from addresses to a register, immediates to a
register, loads and stores using the R constraint
- Other integer modes: accepts immediates to a register, loads and
stores using the R constraint
- DF/SF modes: as you saw before


I have an expand_move that rejects anything not accepted, a check_move
that makes sure everything is ok and the constraints that go with (my
'R' constraint).

The instruction generated during greg should normally be refused.

From what you've said, it seems that I did forget to define something
so I will look at the secondary reload stuff ?

- What exactly is needed there ?

Thanks,
Jc

Re: Constraints and reload

[Joern Rennecke]

Quoting Jean Christophe Beyler <jean.christophe.beyler@gmail.com>:
> I have an expand_move that rejects anything not accepted, a check_move
> that makes sure everything is ok and the constraints that go with (my
> 'R' constraint).
>
> The instruction generated during greg should normally be refused.
>
> From what you've said, it seems that I did forget to define something
> so I will look at the secondary reload stuff ?

When reload decides if something is suitable for reg_equiv_mem or
only for reg_equiv_address, it uses strict_memory_address_p.

If you don't have patterns that can handle certain addresses in DFmode,
you should reject these addresses in GO_IF_LEGITIMATE_ADDRESS /
TARGET_LEGITIMATE_ADDRESS_P.