This is the mail archive of the mailing list for the GCC project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH][M68K] Fix reload failure on cmpdi instruction

Maxim Kuvyrkov wrote:

This patch fixes ICE in reload on the gcc testsuite.

Below is analysis of the problem:

(define_expand "cmpdi"
[(set (cc0)
(compare (match_operand:DI 0 "nonimmediate_operand" "")
(match_operand:DI 1 "general_operand" "")))
(clobber (match_dup 2))])]
"m68k_last_compare_had_fp_operands = 0; operands[2] = gen_reg_rtx (DImode);")

(define_insn ""
  [(set (cc0)
    (compare (match_operand:DI 1 "nonimmediate_operand" "0,d")
         (match_operand:DI 2 "general_operand" "d,0")))
   (clobber (match_operand:DI 0 "register_operand" "=d,d"))]

Here cmpdi expander creates a third operand, that, basically, is a scratch register, and constraints in the instruction pattern specify that one of the original operands should be placed in that register. Reload then failures in find_reloads() on the following instruction (both registers 30 and 31 are pseudos):

(insn 8 7 9 3 (parallel [
(set (cc0)
(compare (reg:DI 30 [ D.1179 ])
(const_double -1 [0xffffffff] 0 [0x0] 0 [0x0] 0 [0x0] 0 [0x0] 0 [0x0])))
(clobber (reg:DI 31))
]) 12 {*} (nil))

What reload needs here to do is to generate move of const_double to register 31, but, for some reason, it fails.

I speculate that the fact that the additional operand is not trivially dead puts additional pressure on reload, which, combined with restrictions of m68k being cc0 target (reload can't use output_reloads for this pattern as it sets cc0) makes reload to fail.

The fix I come up with is to rewrite the expander and the pattern to use (match_scratch) instead. It seems easier for reload to replace (scratch) than to replace a pseudo with a hard register. And, in the context of cmpdi instruction, pseudo and scratch are equivalent.

Tested on ColdFire bare-metal with no regression (gcc, g++ and libstdc++ testsuites). Ok for trunk?
ISTM that we might be papering over a register allocation/reloading problem. This is an area that has been particularly problematical for IRA. So while I'm absolutely in favor of using the match_scratch approach, I'd like to first verify that we're doing the right thing in IRA.

Can you pass along the debugging dumps and a reference to the test which is failing?


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]