cris: For expanded movsi, don't match operands we know will be reloaded
authorHans-Peter Nilsson <hp@axis.com>
Tue, 1 Feb 2022 23:00:10 +0000 (00:00 +0100)
committerHans-Peter Nilsson <hp@axis.com>
Wed, 2 Feb 2022 00:20:03 +0000 (01:20 +0100)
In a session investigating unexpected fallout from a change, I
noticed reload needs one operand being a register to make an
informed decision.  It can happen that there's just a constant
and a memory operand, as in:

(insn 668 667 42 104 (parallel [
            (set (mem:SI (plus:SI (reg/v/f:SI 347 [ fs ])
                        (const_int 168 [0xa8])) \
 [1 fs_126(D)->regs.cfa_how+0 S4 A8])
                (const_int 2 [0x2]))
            (clobber (reg:CC 19 dccr))
        ]) "<...>/gcc/libgcc/unwind-dw2.c":1121:21 22 {*movsi_internal}
     (expr_list:REG_UNUSED (reg:CC 19 dccr)
        (nil)))

This was helpfully created by combine.  When this happens,
reload can't check for costs and preferred register classes,
(both operands will start with NO_REGS as the preferred class)
and will default to the constraints order in the insn in reload.
(Which also does its own temporary merge in find_reloads, but
that's a different story.)  Better don't match the simple cases.
Beware that subregs have to be matched.

I'm doing this just for word_mode (SI) for now, but may repeat
this for the other valid modes as well.  In particular, that
goes for DImode as I see the expanded movdi does *almost* this,
but uses register_operand instead of REG_S_P (from cris.h).
Using REG_S_P is the right choice here because register_operand
also matches (subreg (mem ...)  ...) *until* reload is done.
By itself it's just a sub-0.1% performance win (coremark).

Also removing a stale comment.

gcc:
* config/cris/cris.md ("*movsi_internal<setcc><setnz><setnzvc>"):
Conditionalize on (sub-)register operands or operand 1 being 0.

gcc/config/cris/cris.md

index bc8d758..9d1c179 100644 (file)
     (match_operand:SI 1 "general_operand"
                       "r,Q>,M,M, I,r, M,n,g,r,x,  rQ>,x,gi"))
    (clobber (reg:CC CRIS_CC0_REGNUM))]
-    ;; Note that we prefer not to use the S alternative (if for some reason
-    ;; it competes with others) above, but g matches S.
-  ""
+  ;; Avoid matching insns we know must be reloaded.  Without one
+  ;; operand being a (pseudo-)register, reload chooses
+  ;; reload-registers suboptimally.
+  "REG_S_P (operands[0]) || REG_S_P (operands[1]) || operands[1] == const0_rtx"
 {
   /* Better to have c-switch here; it is worth it to optimize the size of
      move insns.  The alternative would be to try to find more constraint