Re: defining add in a new port
On 01/28/2011 06:44 PM, Ian Lance Taylor wrote:
> Jean-Marc Saffroy writes:
>
>> error: insn does not satisfy its constraints:
>> (insn 1424 1423 141 (set (reg:DI 2 r2)
>> (plus:DI (reg:DI 2 r2)
>> (const_int 40 [0x28])))
>> /home/jmsaffroy/cygnus/src/newlib/libc/time/strptime.c:165 24 {adddi3}
>> (expr_list:REG_EQUIV (plus:DI (reg/f:DI 70 a6)
>> (const_int 40 [0x28]))
>> (nil)))
>
> You should find out what is creating this insn. Is it being created by
> reload, or is it being created by some pass that runs after reload?
With gcc -da, I see that it appears in dump .195r.ira, so that's reload,
right?
> It is likely that you need to make adddi3 a define_expand which tests
> reload_in_progress and reload_completed. If those are the case, you
> will need to explicitly convert
> (set (reg:DI DREG1) (plus:DI (reg:DI DREG2) (const_int N)))
> into
> (set (reg:DI DREG1) (const_int N))
> (set (reg:DI DREG1) (plus:DI (reg:DI DREG1) (REG:DI DREG2)))
>
Ah, but here it happens that DREG1 and DREG2 (r2 and a6 above) are
different types of registers, and I can't add them directly. And since
at this point I won't be allowed to allocate any reg (right?), I guess I
will have to do something like:
(set DREG2 (plus DREG2 N))
(set DREG1 DREG2)
(set DREG2 (plus DREG2 -N))
And that only works if N isn't too big for adds with DREG2. I hope it
will turn out to be an uncommon case, or can be optimized away in
subsequent passes.
> For a much more sophisticated version of this, see, e.g., addsi3 in the
> ARM target.
>
> Ian
>
Ok, I'll look into it.
Thanks a lot!
JM
Re: defining add in a new port
On 01/28/2011 09:45 PM, Ian Lance Taylor wrote:
> Jean-Marc Saffroy writes:
>
>> On 01/28/2011 06:44 PM, Ian Lance Taylor wrote:
>>> Jean-Marc Saffroy writes:
>>>
>>>> error: insn does not satisfy its constraints:
>>>> (insn 1424 1423 141 (set (reg:DI 2 r2)
>>>> (plus:DI (reg:DI 2 r2)
>>>> (const_int 40 [0x28])))
>>>> /home/jmsaffroy/cygnus/src/newlib/libc/time/strptime.c:165 24 {adddi3}
>>>> (expr_list:REG_EQUIV (plus:DI (reg/f:DI 70 a6)
>>>> (const_int 40 [0x28]))
>>>> (nil)))
>>>
>>> You should find out what is creating this insn. Is it being created by
>>> reload, or is it being created by some pass that runs after reload?
>>
>> With gcc -da, I see that it appears in dump .195r.ira, so that's reload,
>> right?
>
> Right.
>
>
>>> It is likely that you need to make adddi3 a define_expand which tests
>>> reload_in_progress and reload_completed. If those are the case, you
>>> will need to explicitly convert
>>> (set (reg:DI DREG1) (plus:DI (reg:DI DREG2) (const_int N)))
>>> into
>>> (set (reg:DI DREG1) (const_int N))
>>> (set (reg:DI DREG1) (plus:DI (reg:DI DREG1) (REG:DI DREG2)))
>>>
>>
>> Ah, but here it happens that DREG1 and DREG2 (r2 and a6 above) are
>> different types of registers, and I can't add them directly.
>
> The insn you showed is adding a constant to a DREG. There is no
> addition of a DREG and an AREG, and I would not expect reload to
> generate any such addition either. Are you looking at a different insn?
> Don't get confused by the REG_EQUIV note, it's irrelevant here.
Actually, looking at the full dumps, I see that this incorrect insn
shown in the ICE above is emitted when eliminating the arg pointer: a6
is the frame pointer (a6+40 probably points to the args on stack).
Running cc1 under gdb, I see that gen_reload is tasked with reloading
(plus AREG N) into (DREG): the first attempts fail (the generated insns
are rejected by emit_insn_if_valid_for_reload), and so gen_reload
recurses (reload.c:8550), generates a 2-insn sequence that does:
(set DREG AREG)
(set DREG (plus DREG N))
But this N is not a valid immediate for an add to a DREG, according to
the predicates I defined for adddi3, which are not checked at this
point, but only later on in final_scan_insn.
So it seems I will have to have adddi3 be a define_expand that splits
the increment by N into smaller increments when called with
(reload_in_progress||reload_completed). Does that sound reasonable?
It's worth noting that I'm developing with an old svn checkout of the
gcc trunk from last October, I will update ASAP just in case.
Jean-Marc
Re: defining add in a new port
On 02/09/2011 08:27 AM, Hans-Peter Nilsson wrote: > On Fri, 28 Jan 2011, Jean-Marc Saffroy wrote: >> (define_constraint "I" >> "Signed 6-bit integer constant for binops." >> (and (match_code "const_int") >>(match_test "IN_RANGE (ival, -24, 32)"))) >> >> (define_register_constraint "A" "ADDR_REGS" >> "The address registers.") >> >> (define_register_constraint "D" "DATA_REGS" >> "The general (data) registers.") >> >> (define_predicate "reg_or_18bit_signed_operand" >> (if_then_else (match_code "const_int") >> (match_test "IN_RANGE (INTVAL (op), -(1 << 17), (1 << 17) - 1)") >> (match_operand 0 "register_operand"))) >> >> (define_insn "adddi3" >> [(set (match_operand:DI 0 "register_operand" "=D,D,A") >> (plus:DI >> (match_operand:DI 1 "register_operand" "%0,0,0") >> (match_operand:DI 2 "reg_or_18bit_signed_operand" "I,D,n")))] >> "" >> "@ >>addi %0, %2 >>add%0, %2 >>adda %0, %2") > >> It seems I was expecting too much intelligence from reload, or I didn't >> give enough hints. > > JFTR (after reading subsequent messages and good pragmatic > advice), I think you're working around a bug in gcc; your adddi3 > should work as-is above. Reload may create suboptimal > sequences, but it should always create working code. Not that > we can do much about it, the port not in the tree and all, but > perhaps later. There are recent-ish changes in this area (i.e. > fp-sp elimination), fixing bugs exposed by other ports, so try > updating your tree too. FTR: at last I could update my tree to a recent SVN checkout of the trunk (from 2 days ago), and I still need a define_expand with the "pragmatic advice" suggested by Ian (which seems to work fine). Cheers, JM > > brgds, H-P >
Re: pattern problem with register assignment
On 02/23/2011 09:52 PM, Christian Grössler wrote:
> Hello,
>
> I have a problem with register allocation. Our architecture has some
> pointer registers "pX" (24bit)
> and some data registers "dX" (32bit). Since pointers are only 24bit,
> we're using PSImode for them.
>
> There are restrictions in the "add" opcode, we can do
>
> pX = add(pX,)
> pX = add(pX,dX)
> pX = add(dX,pX)
> dX = add(pX,pX)
> dX = add(pX,)
> dX = add(dX,dX)
> dX = add(dX,)
> dX = add(pX,dX)
> dX = add(dX,pX)
>
> pX is allowed in at most 2 of the registers involved, but not in all 3
> of them.
> The pattern to add 2 PSImode values looks like this:
>
> (define_insn "*addpsi3"
> [(set (match_operand:PSI 0 "p_d_operand"
> "=a,d0d3,?d0d9 m,?d0d9 m")
> (plus:PSI (match_operand:PSI 1 "p_d_general_operand"
> "%a,d0d3,d0d9 a m,d0d9 a i")
> (match_operand:PSI 2 "p_d_general_operand" "i d0d9 m,d0d3 m
> i,d0d9 i a,d0d9 a m")))]
> ""
> "%0=add(%1,%2)"
> )
>
> It worked fine in the gcc version from 2 years ago, but I'm updating the
> port to current gcc, and
> I get a testsuite failure (one of many :-)) in
> gcc.c-torture/compile/20080812-1.c:
>
> 20080812-1.c: In function 'foo':
> 20080812-1.c:21:1: error: insn does not satisfy its constraints:
> (insn 49 76 77 3 (set (reg:PSI 6 p2 [144])
> (plus:PSI (reg:PSI 5 p1 [orig:145 ivtmp.1 ] [145])
> (reg:PSI 7 p3))) 20080812-1.c:15 193 {*addpsi3}
> (nil))
> 20080812-1.c:21:1: internal compiler error: in
> reload_cse_simplify_operands, at postreload.c:401
> Please submit a full bug report,
> with preprocessed source if appropriate.
> See for instructions.
>
>
> It seems that gcc wants to create an instruction like "p2=add(p1,p3)".
> How can I tell him not to do that?
> I tried to fiddle with the "p_d_general_operand" predication and use a
> modified one for operand 2, but
> at the time the constraint is called, I only see pseudo registers, and
> don't know in which hard register
> they will appear at the end.
>
> I'm using a gcc snapshot from Jan-19-2011.
>
> regards,
> chris
>
>
FWIW, I faced a similar challenge for my private port (ie. define add
for different types of registers), and I got gcc to work by defining a
single pattern for all adds, with predicates that don't discriminate on
register classes: the register contraints do the job of selecting the
proper combinations.
But there is probably more than one way to do it.
Cheers,
JM
defining add in a new port
Hi gcc gurus,
I'm trying to port GCC to a new architecture, I'm new to gcc, and have
little problems defining add correctly.
My target has 2 types of (DI mode) registers, so I defined 2 classes:
- class D (data) regs can be used for computations, and that includes
operations such as additions and increments:
reg += small immediate
or reg1 += reg2
- class A (address) can be used as base for loads, also they can be
modified with moves (between A-regs and from/to D-regs, not from
immediates) and with increments:
reg += large immediate
So first for adddi3 I have defined the following:
(define_constraint "I"
"Signed 6-bit integer constant for binops."
(and (match_code "const_int")
(match_test "IN_RANGE (ival, -24, 32)")))
(define_register_constraint "A" "ADDR_REGS"
"The address registers.")
(define_register_constraint "D" "DATA_REGS"
"The general (data) registers.")
(define_predicate "reg_or_18bit_signed_operand"
(if_then_else (match_code "const_int")
(match_test "IN_RANGE (INTVAL (op), -(1 << 17), (1 << 17) - 1)")
(match_operand 0 "register_operand")))
(define_insn "adddi3"
[(set (match_operand:DI 0 "register_operand" "=D,D,A")
(plus:DI
(match_operand:DI 1 "register_operand" "%0,0,0")
(match_operand:DI 2 "reg_or_18bit_signed_operand" "I,D,n")))]
""
"@
addi %0, %2
add%0, %2
adda %0, %2")
But this doesn't work, I get:
error: insn does not satisfy its constraints:
(insn 1424 1423 141 (set (reg:DI 2 r2)
(plus:DI (reg:DI 2 r2)
(const_int 40 [0x28])))
/home/jmsaffroy/cygnus/src/newlib/libc/time/strptime.c:165 24 {adddi3}
(expr_list:REG_EQUIV (plus:DI (reg/f:DI 70 a6)
(const_int 40 [0x28]))
(nil)))
(r2 is a D-reg, a6 is an A-reg.)
It seems I was expecting too much intelligence from reload, or I didn't
give enough hints.
So I tried separating the add instructions for A-regs and D-regs into
different patterns with more restrictive predicates for each pattern (so
add for A regs can have large increments), but then I get other ICEs
which I don't understand (first assert in elimination_costs_in_insn fails).
Does anyone have a hint how I should approach this? Is a define_expand
required, or something else?
Cheers,
Jean-Marc
