Re: [PATCH, MIPS] Merge seb/seh into regular sign-extend pattern

[Richard Sandiford <rdsandiford at googlemail dot com>]

Adam Nemet <anemet@caviumnetworks.com> writes:
> I am working on a patch to generate dext for zero-extension on MIPS64r2.  This
> is very similar generating seb/seh for sign-extension.  While implementing
> that I was surprised to see that seb/seh weren't generated from the regular
> sign-extend define_insn_and_split.  This patch does that.

Hmm.  If this is purely a stylistic change, then TBH the current version
seems a little clearer and more consistent.  The new version has to
check in three places whether we actually have an SEB/SEH instruction,
and it still leaves the GENERATE_MIPS16E case for a separate pattern,
even though it's very similar in style to the non-MIPS16 SEB/SEH case.

I'm happy to consider it if it is needed by your later patch.
It just doesn't seem like an improvement on its own.

If you do want to combine the patterns, another alternative might be to
use the "enabled" attribute, which wasn't available when these patterns
(or most of the others in mips.md) were written.  E.g.:

;; should go at the end of the attribute list, before the asm attributes.
(define_attr "enabled" ""
  (cond [(eq_attr "type" "signext")
    	 (symbol_ref "ISA_HAS_SEB_SEH")]
        (const_int 1)))

(define_insn_and_split "*extend<SHORT:mode><GPR:mode>2"
  [(set (match_operand:GPR 0 "register_operand" "=d,d,d")
        (sign_extend:GPR
	     (match_operand:SHORT 1 "nonimmediate_operand" "d,d,m")))]
  "!GENERATE_MIPS16E"
  "@
   se<SHORT:size>\t%0,%1
   #
   l<SHORT:size>\t%0,%1"
  "&& reload_completed && REG_P (operands[1]) && !ISA_HAS_SEB_SEH""
  [(set (match_dup 0) (ashift:GPR (match_dup 1) (match_dup 2)))
   (set (match_dup 0) (ashiftrt:GPR (match_dup 0) (match_dup 2)))]
{
  operands[1] = gen_lowpart (<GPR:MODE>mode, operands[1]);
  operands[2] = GEN_INT (GET_MODE_BITSIZE (<GPR:MODE>mode)
			 - GET_MODE_BITSIZE (<SHORT:MODE>mode));
}
  [(set_attr "move_type" "signext,shift_shift,load")
   (set_attr "mode" "<GPR:MODE>")])

which at least cuts the checks down from 3N to N+1.  With a bit of work
we could include the MIPS16e version too.  But whether we include
MIPS16 or not, I suppose the drawback of this approach is that there's
even more magic going on under the hood.

Richard