* gcc-3.4.0 fails for ColdFire(does not satisfy constraints) @ 2004-04-26 20:23 Peter Barada 2004-04-26 20:34 ` Ian Lance Taylor 0 siblings, 1 reply; 15+ messages in thread From: Peter Barada @ 2004-04-26 20:23 UTC (permalink / raw) To: gcc I'm trying to use gcc-3.4.0 --target=m68k-uclinux, and it fails while trhing to build printf.c from uClibc: m68k-uclinux-gcc -Wall -Wstrict-prototypes -Wno-trigraphs -fno-strict-aliasing -Os -Wa,--bitwise-or -I/home/peter/work/src/ucLinux/ucTools/linux-2.4.x/include -m5200 -msoft-float -fno-builtin -nostdinc -D_LIBC -I../../include -I. -I/home/mylocal/uclinux/tools-3.4.0/lib/gcc/m68k-uclinux/3.4.0/include -DNDEBUG -DL__fpmaxtostr printf.c -c -o _fpmaxtostr.o printf.c: In function `_fpmaxtostr': printf.c:2454: error: insn does not satisfy its constraints: (insn 1376 536 537 38 (set (reg:QI 8 %a0) (mem:QI (plus:SI (reg/f:SI 14 %a6) (const_int -209 [0xffffff2f])) [0 mode+0 S1 A8])) 33 {*m68k.md:826} (nil) (nil)) printf.c:2454: internal compiler error: in reload_cse_simplify_operands, at postreload.c:378 Please submit a full bug report, with preprocessed source if appropriate. See <URL:http://gcc.gnu.org/bugs.html> for instructions. make[2]: *** [_fpmaxtostr.o] Error 1 make[2]: Leaving directory `/home/peter/work/src/ucLinux/ucTools/uClibc-0.9.26/libc/stdio' This problem has been reported before, so I decided to try to fix it by preventing bytes from being stored in address registers by modifying the movqi pattern for ColdFire from : (define_insn "" [(set (match_operand:QI 0 "nonimmediate_operand" "=d<Q>,dm,U,d*a") (match_operand:QI 1 "general_src_operand" "dmi,d<Q>,U,di*a"))] "TARGET_COLDFIRE" "* return output_move_qimode (operands);") to: (define_insn "" [(set (match_operand:QI 0 "nonimmediate_operand" "=d<Q>,dm,U,d") (match_operand:QI 1 "general_src_operand" "dmi,d<Q>,U,di"))] "TARGET_COLDFIRE" "* return output_move_qimode (operands);") As well as modify HARD_REGNO_MODE_OK from: #define HARD_REGNO_MODE_OK(REGNO, MODE) \ (((REGNO) < 16 \ && !((REGNO) < 8 && (REGNO) + GET_MODE_SIZE (MODE) / 4 > 8)) \ || ((REGNO) >= 16 && (REGNO) < 24 \ && (GET_MODE_CLASS (MODE) == MODE_FLOAT \ || GET_MODE_CLASS (MODE) == MODE_COMPLEX_FLOAT) \ && GET_MODE_UNIT_SIZE (MODE) <= 12)) To: #define HARD_REGNO_MODE_OK(REGNO, MODE) \ regno_mode_ok(REGNO, MODE) along with the following code: /* Value is 1 if hard register REGNO can hold a value of machine-mode MODE. On the 68000, the cpu registers can hold any mode except bytes in address registers, but the 68881 registers can hold only SFmode or DFmode. */ int regno_mode_ok(int regno, enum machine_mode mode) { if (regno < 8) { /* Data Registers */ return 1; } else if (regno < 16) { /* Address Registers, can't hold bytes, can hold aggregate if fits in */ if (GET_MODE_SIZE (mode) == 1) return 0; if (!((regno) < 8 && (regno) + GET_MODE_SIZE (mode) / 4 > 8)) return 1; } else if (regno < 24) { /* FPU registers, hold float or complex float of long double or smaller */ if ((GET_MODE_CLASS (mode) == MODE_FLOAT || GET_MODE_CLASS (mode) == MODE_COMPLEX_FLOAT) && GET_MODE_UNIT_SIZE (mode) <= 12) return 0; } return 0; } Now fails with: printf.c: In function `_fpmaxtostr': printf.c:2454: error: unable to find a register to spill in class `ADDR_REGS' printf.c:2454: error: this is the insn: (insn 537 536 538 38 (set (subreg:SI (reg/v:QI 50 [ mode ]) 0) (plus:SI (subreg:SI (reg/v:QI 50 [ mode ]) 0) (const_int 32 [0x20]))) 90 {*addsi3_5200} (nil) (nil)) Breakpoint 1, fancy_abort ( file=0x82582e0 "/home/peter/work/src/ucLinux/ucTools/gcc-3.4.0/gcc/reload1.c", line=1874, function=0x82580f7 "spill_failure") at /home/peter/work/src/ucLinux/ucTools/gcc-3.4.0/gcc/diagnostic.c:584 Any suggestions where I go from here are most appreciated. -- Peter Barada peter@the-baradas.com ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: gcc-3.4.0 fails for ColdFire(does not satisfy constraints) 2004-04-26 20:23 gcc-3.4.0 fails for ColdFire(does not satisfy constraints) Peter Barada @ 2004-04-26 20:34 ` Ian Lance Taylor 2004-04-26 21:06 ` Peter Barada 0 siblings, 1 reply; 15+ messages in thread From: Ian Lance Taylor @ 2004-04-26 20:34 UTC (permalink / raw) To: Peter Barada; +Cc: gcc Peter Barada <peter@the-baradas.com> writes: > Now fails with: > > printf.c: In function `_fpmaxtostr': > printf.c:2454: error: unable to find a register to spill in class `ADDR_REGS' > printf.c:2454: error: this is the insn: > (insn 537 536 538 38 (set (subreg:SI (reg/v:QI 50 [ mode ]) 0) > (plus:SI (subreg:SI (reg/v:QI 50 [ mode ]) 0) > (const_int 32 [0x20]))) 90 {*addsi3_5200} (nil) > (nil)) > > Breakpoint 1, fancy_abort ( > file=0x82582e0 "/home/peter/work/src/ucLinux/ucTools/gcc-3.4.0/gcc/reload1.c", > line=1874, function=0x82580f7 "spill_failure") > at /home/peter/work/src/ucLinux/ucTools/gcc-3.4.0/gcc/diagnostic.c:584 > > > Any suggestions where I go from here are most appreciated. I think INDEX_REG_CLASS looks a bit fishy to me. GENERAL_REGS is fine for 68020, but is it really right for 68000 or Coldfire? Perhaps it should be DATA_REGS? Actually, I don't understand the abort. The last alternative for the insn permits r,0,r, which can be matched by reloading the constant 32 into any register, so why is the code trying to spill ADDR_REGS? Why not GENERAL_REGS? Using mrIKLs in a constraint is tricky stuff, anyhow. It can often lead to better code by causing gcc to use moveq to load some other register, and then do the addition. But that increases register pressure; if no other register is available, then the resulting code is actually worse. It might be better to not use the 's' constraint, but to instead use a peephole2 which uses match_scratch, a technique which was not available when the m68k backend was written. Ian ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: gcc-3.4.0 fails for ColdFire(does not satisfy constraints) 2004-04-26 20:34 ` Ian Lance Taylor @ 2004-04-26 21:06 ` Peter Barada 2004-04-26 21:10 ` Ian Lance Taylor 0 siblings, 1 reply; 15+ messages in thread From: Peter Barada @ 2004-04-26 21:06 UTC (permalink / raw) To: ian; +Cc: gcc >> Now fails with: >> >> printf.c: In function `_fpmaxtostr': >> printf.c:2454: error: unable to find a register to spill in class `ADDR_REGS' >> printf.c:2454: error: this is the insn: >> (insn 537 536 538 38 (set (subreg:SI (reg/v:QI 50 [ mode ]) 0) >> (plus:SI (subreg:SI (reg/v:QI 50 [ mode ]) 0) >> (const_int 32 [0x20]))) 90 {*addsi3_5200} (nil) >> (nil)) >> >> Breakpoint 1, fancy_abort ( >> file=0x82582e0 "/home/peter/work/src/ucLinux/ucTools/gcc-3.4.0/gcc/reload1.c", >> line=1874, function=0x82580f7 "spill_failure") >> at /home/peter/work/src/ucLinux/ucTools/gcc-3.4.0/gcc/diagnostic.c:584 >> >> >> Any suggestions where I go from here are most appreciated. > >I think INDEX_REG_CLASS looks a bit fishy to me. GENERAL_REGS is fine >for 68020, but is it really right for 68000 or Coldfire? Perhaps it >should be DATA_REGS? ColdFire allows for d8(Ax,Xi*SF) where Xi is either an address or data register, abd the scalefactor(SF) is either 1,2,4, or 8 if the FPU is attached, so INDEX_REG_CLASS looks right, at least for ColdFire. >Using mrIKLs in a constraint is tricky stuff, anyhow. It can often >lead to better code by causing gcc to use moveq to load some other >register, and then do the addition. But that increases register >pressure; if no other register is available, then the resulting code >is actually worse. It might be better to not use the 's' constraint, >but to instead use a peephole2 which uses match_scratch, a technique >which was not available when the m68k backend was written. The constraint is limited to: (define_insn "" [(set (match_operand:QI 0 "nonimmediate_operand" "=d<Q>,dm,d") (match_operand:QI 1 "general_src_operand" "dmi,d<Q>,di"))] "TARGET_COLDFIRE" "* return output_move_qimode (operands);") Which doesn't allow for IKLs. This constraint *should* work, right? Which ColdFire pattern are you seeing 'mrIKLs' on that you think should be changed? -- Peter Barada peter@the-baradas.com ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: gcc-3.4.0 fails for ColdFire(does not satisfy constraints) 2004-04-26 21:06 ` Peter Barada @ 2004-04-26 21:10 ` Ian Lance Taylor 2004-04-26 21:24 ` Peter Barada 0 siblings, 1 reply; 15+ messages in thread From: Ian Lance Taylor @ 2004-04-26 21:10 UTC (permalink / raw) To: Peter Barada; +Cc: gcc Peter Barada <peter@the-baradas.com> writes: > >Using mrIKLs in a constraint is tricky stuff, anyhow. It can often > >lead to better code by causing gcc to use moveq to load some other > >register, and then do the addition. But that increases register > >pressure; if no other register is available, then the resulting code > >is actually worse. It might be better to not use the 's' constraint, > >but to instead use a peephole2 which uses match_scratch, a technique > >which was not available when the m68k backend was written. > > The constraint is limited to: > > (define_insn "" > [(set (match_operand:QI 0 "nonimmediate_operand" "=d<Q>,dm,d") > (match_operand:QI 1 "general_src_operand" "dmi,d<Q>,di"))] > "TARGET_COLDFIRE" > "* return output_move_qimode (operands);") > > Which doesn't allow for IKLs. This constraint *should* work, right? > Which ColdFire pattern are you seeing 'mrIKLs' on that you think should be > changed? addsi3_5200, which is the one which crashed reload in your last message. Ian ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: gcc-3.4.0 fails for ColdFire(does not satisfy constraints) 2004-04-26 21:10 ` Ian Lance Taylor @ 2004-04-26 21:24 ` Peter Barada 2004-04-26 22:17 ` Ian Lance Taylor 0 siblings, 1 reply; 15+ messages in thread From: Peter Barada @ 2004-04-26 21:24 UTC (permalink / raw) To: ian; +Cc: gcc >> Which doesn't allow for IKLs. This constraint *should* work, right? >> Which ColdFire pattern are you seeing 'mrIKLs' on that you think should be >> changed? > >addsi3_5200, which is the one which crashed reload in your last >message. Ah, I've been staring at RTL dumps too long. I was focusing on the move, not the add :) I changed the constraint from 's' to 'i': (define_insn "*addsi3_5200" [(set (match_operand:SI 0 "nonimmediate_operand" "=m,?a,?a,r") (plus:SI (match_operand:SI 1 "general_operand" "%0,a,rJK,0") (match_operand:SI 2 "general_src_operand" "d,rJK,a,mrIKLi")))] "TARGET_COLDFIRE" "* return output_addsi3 (operands);") and the problem has gone away. The resultant code that failed before looks like : |(insn 1373 532 533 (set (reg:QI 0 %d0) | (mem:QI (plus:SI (reg/f:SI 14 %a6) | (const_int -112 [0xffffff90])) [0 S1 A8])) 33 {*m68k.md:826} (nil) | (nil)) move.b -112(%a6),%d0 |, | 1373 *m68k.md:826/1 |(insn 533 1373 534 (set (reg:SI 0 %d0 [177]) | (plus:SI (reg:SI 0 %d0) | (const_int -32 [0xffffffe0]))) 90 {*addsi3_5200} (nil) | (nil)) add.l #-32,%d0 |, tmp177 | 533 *addsi3_5200/4 |(insn 534 533 536 (set (mem:QI (plus:SI (reg/f:SI 14 %a6) | (const_int -112 [0xffffff90])) [0 S1 A8]) | (reg:QI 0 %d0 [orig:177+3 ] [177])) 33 {*m68k.md:826} (insn_list 533 (nil)) | (expr_list:REG_DEAD (reg:QI 0 %d0 [orig:177+3 ] [177]) | (nil))) move.b %d0,-112(%a6) | tmp177, | 534 *m68k.md:826/2 Which is about as good as it can get on ColdFire since there is no valid addqi3 pattern. > >is actually worse. It might be better to not use the 's' constraint, > >but to instead use a peephole2 which uses match_scratch, a technique > >which was not available when the m68k backend was written. Which backend would be a good place to find examples of this type of peephole2 pattern? What is it that the pattern should do, create a scratch register to stuff the constant? Are peephole2 patterns run after reload? -- Peter Barada peter@the-baradas.com ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: gcc-3.4.0 fails for ColdFire(does not satisfy constraints) 2004-04-26 21:24 ` Peter Barada @ 2004-04-26 22:17 ` Ian Lance Taylor 2004-04-27 0:53 ` Peter Barada 0 siblings, 1 reply; 15+ messages in thread From: Ian Lance Taylor @ 2004-04-26 22:17 UTC (permalink / raw) To: Peter Barada; +Cc: gcc Peter Barada <peter@the-baradas.com> writes: > Which is about as good as it can get on ColdFire since there is no > valid addqi3 pattern. Ah, now I see why reload was looking for ADDR_REG. Because addsi3_5200 has a ?a,a,rJK alternative, and -32 matches the J constraint. So I do think the problem is in that insn. It will tend to encourage the use of an address register in some cases for which a general register is appropriate. > > >is actually worse. It might be better to not use the 's' constraint, > > >but to instead use a peephole2 which uses match_scratch, a technique > > >which was not available when the m68k backend was written. > > Which backend would be a good place to find examples of this type of > peephole2 pattern? I dunno, anything really. See the docs. Here's a simple one from i386.md; i386.md is probably relevant since it is also CISC: (define_peephole2 [(set (match_operand:SI 0 "push_operand" "") (match_operand:SI 1 "memory_operand" "")) (match_scratch:SI 2 "r")] "! optimize_size && ! TARGET_PUSH_MEMORY" [(set (match_dup 2) (match_dup 1)) (set (match_dup 0) (match_dup 2))] "") This says that if we are trying to push a value loaded from memory, and we happen to have a free register (as indicated by match_scratch), then optimize to load the register from memory, and then push the register. The resulting two simple instructions are presumably no slower than the first complex instruction. This then permits the load and the push to be scheduled independently. > What is it that the pattern should do, create a > scratch register to stuff the constant? You can use match_scratch to pick up a free register. If all the relevant registers are in use, the peephole will not trigger. > Are peephole2 patterns run > after reload? Yes. They are run after reload, but before the second instruction scheduling pass. Ian ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: gcc-3.4.0 fails for ColdFire(does not satisfy constraints) 2004-04-26 22:17 ` Ian Lance Taylor @ 2004-04-27 0:53 ` Peter Barada 2004-04-27 4:20 ` Ian Lance Taylor 0 siblings, 1 reply; 15+ messages in thread From: Peter Barada @ 2004-04-27 0:53 UTC (permalink / raw) To: ian; +Cc: gcc >> Which is about as good as it can get on ColdFire since there is no >> valid addqi3 pattern. > >Ah, now I see why reload was looking for ADDR_REG. Because >addsi3_5200 has a ?a,a,rJK alternative, and -32 matches the J >constraint. So I do think the problem is in that insn. It will tend >to encourage the use of an address register in some cases for which a >general register is appropriate. Why is 'JK' on constraints of both operands for '?a,rJK,rJK'? SHould that be 'r,0,rJK'? Should I rearrange the constraints to put 'r,0,mrIi' ahead of the last two reload constraints as in: (define_insn "*addsi3_5200" [(set (match_operand:SI 0 "nonimmediate_operand" "=m,r,?a,?a") (plus:SI (match_operand:SI 1 "general_operand" "%0,0,a,rJK") (match_operand:SI 2 "general_src_operand" "d,mrIi,rJK")))] "TARGET_COLDFIRE" "* return output_addsi3 (operands);") Would this problem also exist for subsi3: (define_insn "subsi3" [(set (match_operand:SI 0 "nonimmediate_operand" "=m,d,a") (minus:SI (match_operand:SI 1 "general_operand" "0,0,0") (match_operand:SI 2 "general_src_operand" "dT,mSrT,mSrs")))] "" "sub%.l %2,%0") where the last constraint should add 'i'? -- Peter Barada peter@the-baradas.com ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: gcc-3.4.0 fails for ColdFire(does not satisfy constraints) 2004-04-27 0:53 ` Peter Barada @ 2004-04-27 4:20 ` Ian Lance Taylor 2004-04-27 8:46 ` Jim Wilson 2004-04-27 14:57 ` Peter Barada 0 siblings, 2 replies; 15+ messages in thread From: Ian Lance Taylor @ 2004-04-27 4:20 UTC (permalink / raw) To: Peter Barada; +Cc: gcc Peter Barada <peter@the-baradas.com> writes: > >Ah, now I see why reload was looking for ADDR_REG. Because > >addsi3_5200 has a ?a,a,rJK alternative, and -32 matches the J > >constraint. So I do think the problem is in that insn. It will tend > >to encourage the use of an address register in some cases for which a > >general register is appropriate. Here is the current insn for reference: (define_insn "*addsi3_5200" [(set (match_operand:SI 0 "nonimmediate_operand" "=m,?a,?a,r") (plus:SI (match_operand:SI 1 "general_operand" "%0,a,rJK,0") (match_operand:SI 2 "general_src_operand" "d,rJK,a,mrIKLs")))] "TARGET_COLDFIRE" "* return output_addsi3 (operands);") Alternative 0 is memory += data register. Alternative 1 is address register plus address register plus (register or signed 16 bit constant or any constant which moveq can't handle). Alternative 2 is (register or signed 16 bit constant or any constant which moveq can't handle) plus address register. Alternative 3 is any register += (memory or any register or addq constant or any constant which moveq can't handle or subq constant or unknown constant). I dont know why alternatives 1 and 2 both exist. Since operand 1 is commutative, there should be no need to have both alternatives. I don't know why alternatives 1 and 2 have both 'J' and 'K'. Every integer value matches either 'J' or 'K'. Specifying "rJK" is the same as specifying "ri". I tracked back through CVS history, and the constraints have looked like that since 1992. Looking through some old releases, I see that in version 1.33, in 1989, the addsi3 insn looked like this: (define_insn "addsi3" [(set (match_operand:SI 0 "general_operand" "=m,r,!a") (plus:SI (match_operand:SI 1 "general_operand" "%0,0,a") (match_operand:SI 2 "general_operand" "dIKLs,mrIKLs,rJK")))] So the "rJK" has existed since then. I have no idea why this was written in this way. The duplicated commutative version was added with this ChangeLog entry: Sat Feb 18 02:11:25 1989 Richard Stallman (rms at sugar-bombs.ai.mit.edu) * m68k.md, alliant.md (addsi3): New alternative a=r+a, in addition to old a=a+r, needed since reload-insns are not commuted. As far as I know this is no longer true, and the extra alternative is no longer required. > Why is 'JK' on constraints of both operands for '?a,rJK,rJK'? Well, if I understand you correctly, there is no "?a,rJK,rJK". There is "?a,a,rJK", and "?a,rJK,a". > SHould > that be 'r,0,rJK'? No, additions to any register are handled by alternative 3. > Should I rearrange the constraints to put > 'r,0,mrIi' ahead of the last two reload constraints as in: Well, you don't need both 'I' and 'i', since 'i' will match anything that 'I' does. And I don't think the order of the alternatives will matter very much in this case. > Would this problem also exist for subsi3: > > (define_insn "subsi3" > [(set (match_operand:SI 0 "nonimmediate_operand" "=m,d,a") > (minus:SI (match_operand:SI 1 "general_operand" "0,0,0") > (match_operand:SI 2 "general_src_operand" "dT,mSrT,mSrs")))] > "" > "sub%.l %2,%0") > > where the last constraint should add 'i'? Yes, this looks the same to me. The code is probably avoiding 'i' because it is more efficient to load the constant into a register. But again I think it would be better (with today's compiler) to permit 'i', and use a peephole2 to permit using a register when and if there is a register available. In fact, I would guess that each alternative for this insn should support 'i', with appropriate peephole2s, although I would want to doublecheck that you can indeed subtract any immediate value from a memory location. Ian ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: gcc-3.4.0 fails for ColdFire(does not satisfy constraints) 2004-04-27 4:20 ` Ian Lance Taylor @ 2004-04-27 8:46 ` Jim Wilson 2004-04-27 14:57 ` Peter Barada 1 sibling, 0 replies; 15+ messages in thread From: Jim Wilson @ 2004-04-27 8:46 UTC (permalink / raw) To: Ian Lance Taylor; +Cc: gcc, peter Ian Lance Taylor wrote: > * m68k.md, alliant.md (addsi3): New alternative a=r+a, in addition > to old a=a+r, needed since reload-insns are not commuted. > As far as I know this is no longer true, and the extra alternative is > no longer required. We commute when reloading (input) insns, but we don't commute when emitting reload (output) insns. See gen_reload () which generally just emits a reload the same way it was passed to it without checking for commutative operands. This might be the reason why rms made this change. gen_reload does handle the special case where we have 3 register operands and the second PLUS operand matches the output operand. It doesn't handle the case where the first PLUS operand is a constant. Based on normal canonicalization rules, we shouldn't be getting (plus constant reg) here, but maybe there is an unusual case where this can happen. I think it would be reasonable to consider this a reload bug though. Actually, I see that current code handles SUBREG and the old gcc-2.2.2 code does not. Maybe that was the problem. In which case, there still seems to be a bug in the current sourecs, because the the commutative check in gen_reload doesn't handle SUBREGs, so in theory we could still end up with the a=r+a case. It will be rejected, and then we will emit an alternative sequence, so this will still work, but you will silently lose some performance because you get a less efficient reload sequence generated. Again, this probably should be fixed in gen_reload rather than in m68k.md. -- Jim Wilson, GNU Tools Support, http://www.SpecifixInc.com ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: gcc-3.4.0 fails for ColdFire(does not satisfy constraints) 2004-04-27 4:20 ` Ian Lance Taylor 2004-04-27 8:46 ` Jim Wilson @ 2004-04-27 14:57 ` Peter Barada 2004-04-27 15:10 ` Ian Lance Taylor 1 sibling, 1 reply; 15+ messages in thread From: Peter Barada @ 2004-04-27 14:57 UTC (permalink / raw) To: ian; +Cc: gcc >> >Ah, now I see why reload was looking for ADDR_REG. Because >> >addsi3_5200 has a ?a,a,rJK alternative, and -32 matches the J >> >constraint. So I do think the problem is in that insn. It will tend >> >to encourage the use of an address register in some cases for which a >> >general register is appropriate. > >Here is the current insn for reference: > >(define_insn "*addsi3_5200" > [(set (match_operand:SI 0 "nonimmediate_operand" "=m,?a,?a,r") > (plus:SI (match_operand:SI 1 "general_operand" "%0,a,rJK,0") > (match_operand:SI 2 "general_src_operand" "d,rJK,a,mrIKLs")))] > "TARGET_COLDFIRE" > "* return output_addsi3 (operands);") > >Alternative 0 is memory += data register. Alternative 1 is address >register plus address register plus (register or signed 16 bit >constant or any constant which moveq can't handle). Alternative 2 is >(register or signed 16 bit constant or any constant which moveq can't >handle) plus address register. Alternative 3 is any register += >(memory or any register or addq constant or any constant which moveq >can't handle or subq constant or unknown constant). I'm still wondering why replacing 's' with 'i' in the last alternative makes the issue go away, and produces(in this particular case), good code. As for the peephole2 to use moveq to load the constant into a data register, does this look right? (define_peephole2 [(match_scratch:SI 2 "d") (set (match_operand:SI 0 "register_or_memory_operand" "rm") (plus:SI (match_dup 0) (match_operand:SI 1 "immediate_operand" "K")))] "!symbolic_operand (operands[1], SImode)" [(set (match_dup 2) (match_dup 1)) (set (match_dup 0) (plus:SI (match_dup 0) (match_dup 2)))] "") -- Peter Barada peter@the-baradas.com ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: gcc-3.4.0 fails for ColdFire(does not satisfy constraints) 2004-04-27 14:57 ` Peter Barada @ 2004-04-27 15:10 ` Ian Lance Taylor 2004-04-27 16:34 ` Peter Barada 0 siblings, 1 reply; 15+ messages in thread From: Ian Lance Taylor @ 2004-04-27 15:10 UTC (permalink / raw) To: Peter Barada; +Cc: gcc Peter Barada <peter@the-baradas.com> writes: > >Here is the current insn for reference: > > > >(define_insn "*addsi3_5200" > > [(set (match_operand:SI 0 "nonimmediate_operand" "=m,?a,?a,r") > > (plus:SI (match_operand:SI 1 "general_operand" "%0,a,rJK,0") > > (match_operand:SI 2 "general_src_operand" "d,rJK,a,mrIKLs")))] > > "TARGET_COLDFIRE" > > "* return output_addsi3 (operands);") > > > >Alternative 0 is memory += data register. Alternative 1 is address > >register plus address register plus (register or signed 16 bit > >constant or any constant which moveq can't handle). Alternative 2 is > >(register or signed 16 bit constant or any constant which moveq can't > >handle) plus address register. Alternative 3 is any register += > >(memory or any register or addq constant or any constant which moveq > >can't handle or subq constant or unknown constant). > > I'm still wondering why replacing 's' with 'i' in the last alternative > makes the issue go away, and produces(in this particular case), good > code. The constraint 's' matches any integer whose value is not known. A typical example would be the address of a variable. In your example, the value of the second operand was (I think) the constant -32. Since that value is known, the constraint 's' does not match. The other constraints, "mrIKL", also do not match a constant -32. So the last alternative does not match. The constraint 'i', on the other hand, matches any constant integer. So when we change 's' to 'i', the last alternative does match, and gcc uses it. Alternative 2 does match with -32, but that alternatives requires adding an address register to an address register. My guess is that gcc chooses that alternative, and tries to find an address register to use. It appears that, it can't, perhaps because they have all been used for other purposes. That leads to the "unable to find a register to spill in class ADDR_REGS" message. This is a plausible scenario, though it's hard to be certain without the full test case (which I don't particularly want). > As for the peephole2 to use moveq to load the constant into a data > register, does this look right? > > (define_peephole2 > [(match_scratch:SI 2 "d") > (set (match_operand:SI 0 "register_or_memory_operand" "rm") > (plus:SI (match_dup 0) > (match_operand:SI 1 "immediate_operand" "K")))] > "!symbolic_operand (operands[1], SImode)" > [(set (match_dup 2) (match_dup 1)) > (set (match_dup 0) (plus:SI (match_dup 0) (match_dup 2)))] > "") That's not quite what I was thinking of. I was thinking that would match constants which moveq can handle, which is the reverse of the constraints 'K' (if you can't use moveq for the constant, this optimization doesn't really help, as far as I can see). Also, you don't need to check symbolic_operand condition, because the constraint 'K' will only match a CONST_INT anyhow. I would try something like this: (define_peephole2 [(match_scratch:SI 2 "d") (set (match_operand:SI 0 "register_or_memory_operand" "rm") (plus:SI (match_dup 0) (match_operand:SI 1 "immediate_operand" "J")))] "INTVAL (operand[1]) >= -0x80 && INTVAL (operand[1]) < 0x80" [(set (match_dup 2) (match_dup 1)) (set (match_dup 0) (plus:SI (match_dup 0) (match_dup 2)))] "") Ian ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: gcc-3.4.0 fails for ColdFire(does not satisfy constraints) 2004-04-27 15:10 ` Ian Lance Taylor @ 2004-04-27 16:34 ` Peter Barada 2004-04-27 16:35 ` Ian Lance Taylor 0 siblings, 1 reply; 15+ messages in thread From: Peter Barada @ 2004-04-27 16:34 UTC (permalink / raw) To: ian; +Cc: gcc >That's not quite what I was thinking of. I was thinking that would >match constants which moveq can handle, which is the reverse of the >constraints 'K' (if you can't use moveq for the constant, this >optimization doesn't really help, as far as I can see). Also, you >don't need to check symbolic_operand condition, because the constraint >'K' will only match a CONST_INT anyhow. I would try something like >this: > >(define_peephole2 > [(match_scratch:SI 2 "d") > (set (match_operand:SI 0 "register_or_memory_operand" "rm") > (plus:SI (match_dup 0) > (match_operand:SI 1 "immediate_operand" "J")))] > "INTVAL (operand[1]) >= -0x80 && INTVAL (operand[1]) < 0x80" > [(set (match_dup 2) (match_dup 1)) > (set (match_dup 0) (plus:SI (match_dup 0) (match_dup 2)))] > "") I need to ingest more caffeine before I try to write RTL :) I fat-fingered the 'J'. For ColdFire(at least on the newer cores), the immediate add is faster(but larger) than the moveq/add. If there are data registers available, will this peephole(if matched multiple times in a block for the *same* constant), create more than one constant register loaded with moveq, or is the compiler smart enough to coalesce them together? I'll give the following a whirl and see what happens: (define_peephole2 [(match_scratch:SI 2 "d") (set (match_operand:SI 0 "register_or_memory_operand" "rm") (plus:SI (match_dup 0) (match_operand:SI 1 "immediate_operand" "J")))] "optimize_size && INTVAL (operand[1]) >= -0x80 && INTVAL (operand[1]) < 0x80" [(set (match_dup 2) (match_dup 1)) (set (match_dup 0) (plus:SI (match_dup 0) (match_dup 2)))] "") -- Peter Barada peter@the-baradas.com ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: gcc-3.4.0 fails for ColdFire(does not satisfy constraints) 2004-04-27 16:34 ` Peter Barada @ 2004-04-27 16:35 ` Ian Lance Taylor 2004-04-27 16:45 ` Peter Barada 0 siblings, 1 reply; 15+ messages in thread From: Ian Lance Taylor @ 2004-04-27 16:35 UTC (permalink / raw) To: Peter Barada; +Cc: gcc Peter Barada <peter@the-baradas.com> writes: > For ColdFire(at least on the newer cores), the immediate add is > faster(but larger) than the moveq/add. OK, then we don't want to use this for Coldfire. But I suspect that we do want to use it for 680x0. > If there are data registers > available, will this peephole(if matched multiple times in a block for > the *same* constant), create more than one constant register loaded > with moveq, or is the compiler smart enough to coalesce them together? The compiler might coalesce them together. It's not impossible, though it's not likely to do a great job on it. You'll have to see. Ian ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: gcc-3.4.0 fails for ColdFire(does not satisfy constraints) 2004-04-27 16:35 ` Ian Lance Taylor @ 2004-04-27 16:45 ` Peter Barada 2004-04-27 17:32 ` Ian Lance Taylor 0 siblings, 1 reply; 15+ messages in thread From: Peter Barada @ 2004-04-27 16:45 UTC (permalink / raw) To: ian; +Cc: gcc >> For ColdFire(at least on the newer cores), the immediate add is >> faster(but larger) than the moveq/add. > >OK, then we don't want to use this for Coldfire. But I suspect that >we do want to use it for 680x0. For m68k, the pattern is (define_insn "*addsi3_internal" [(set (match_operand:SI 0 "nonimmediate_operand" "=m,?a,?a,d,a") (plus:SI (match_operand:SI 1 "general_operand" "%0,a,rJK,0,0") (match_operand:SI 2 "general_src_operand" "dIKLT,rJK,a,mSrIKLT,mSrIKLs")))] "! TARGET_COLDFIRE" "* return output_addsi3 (operands);") Which should work correctly since it would force a register to be used to hold the constant, and I'm assuming that by seperating the last two alternatives that the compiler will favor a data register over an address register. I'll modify the peephole2 to have TARGET_COLDFIRE in it to prevent it from triggering for m68k. -- Peter Barada peter@the-baradas.com ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: gcc-3.4.0 fails for ColdFire(does not satisfy constraints) 2004-04-27 16:45 ` Peter Barada @ 2004-04-27 17:32 ` Ian Lance Taylor 0 siblings, 0 replies; 15+ messages in thread From: Ian Lance Taylor @ 2004-04-27 17:32 UTC (permalink / raw) To: Peter Barada; +Cc: gcc Peter Barada <peter@the-baradas.com> writes: > >> For ColdFire(at least on the newer cores), the immediate add is > >> faster(but larger) than the moveq/add. > > > >OK, then we don't want to use this for Coldfire. But I suspect that > >we do want to use it for 680x0. > > For m68k, the pattern is > > (define_insn "*addsi3_internal" > [(set (match_operand:SI 0 "nonimmediate_operand" "=m,?a,?a,d,a") > (plus:SI (match_operand:SI 1 "general_operand" "%0,a,rJK,0,0") > (match_operand:SI 2 "general_src_operand" "dIKLT,rJK,a,mSrIKLT,mSrIKLs")))] > > > "! TARGET_COLDFIRE" > "* return output_addsi3 (operands);") > > Which should work correctly since it would force a register to be used > to hold the constant, and I'm assuming that by seperating the last two > alternatives that the compiler will favor a data register over an > address register. It will indeed force a register to be used, but I think that in the general case we do not want to force a register to be used. If we always force a register to be used, then we increase register pressure. If we force a register to be spilled due to constraint failure, then we have generated worse code. The advantage of the peephole2 approach is that it will use a register only if there is one available. If all the registers are being used for something else, then we won't force one of them to be spilled just to hold our constant. Either choice will generate worse code in some situations. My guess is that the peephole2 approach will be better on average. But it should be tested on some benchmarks. Ian ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2004-04-27 15:26 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2004-04-26 20:23 gcc-3.4.0 fails for ColdFire(does not satisfy constraints) Peter Barada 2004-04-26 20:34 ` Ian Lance Taylor 2004-04-26 21:06 ` Peter Barada 2004-04-26 21:10 ` Ian Lance Taylor 2004-04-26 21:24 ` Peter Barada 2004-04-26 22:17 ` Ian Lance Taylor 2004-04-27 0:53 ` Peter Barada 2004-04-27 4:20 ` Ian Lance Taylor 2004-04-27 8:46 ` Jim Wilson 2004-04-27 14:57 ` Peter Barada 2004-04-27 15:10 ` Ian Lance Taylor 2004-04-27 16:34 ` Peter Barada 2004-04-27 16:35 ` Ian Lance Taylor 2004-04-27 16:45 ` Peter Barada 2004-04-27 17:32 ` Ian Lance Taylor
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).