Re: [PATCH, MIPS] Move patterns involving truncate out of "truncate section"

[Richard Sandiford <rdsandiford at googlemail dot com>]

Adam Nemet <anemet@caviumnetworks.com> writes:
> The octeon instruction exts can be used to sign-extend an arbitrary value
> regardless whether the operand is a proper SI value.  IMO, these new patterns
> would belong to sign-extend but right now the existing truncation/sign-extend
> patterns are under the "truncate section".
>
> I think that if an operation produces either a properly truncated value or
> does not require its operands to be proper SI values then that is really the
> property of the operation rather than that of truncate.  Therefore, the patch
> below moves the combiner patterns from the truncate section to the section of
> their other operation (zero_extend, extend and shift).

OK, you've convinced me that this makes sense for truncated inputs.
However, I'm not sure it's clear cut for:

-;; Combiner patterns to optimize shift/truncate combinations.
-
-(define_insn "*ashr_trunc<mode>"
-  [(set (match_operand:SUBDI 0 "register_operand" "=d")
-        (truncate:SUBDI
-	  (ashiftrt:DI (match_operand:DI 1 "register_operand" "d")
-		       (match_operand:DI 2 "const_arith_operand" ""))))]
-  "TARGET_64BIT && !TARGET_MIPS16 && IN_RANGE (INTVAL (operands[2]), 32, 63)"
-  "dsra\t%0,%1,%2"
-  [(set_attr "type" "shift")
-   (set_attr "mode" "<MODE>")])
-
-(define_insn "*lshr32_trunc<mode>"
-  [(set (match_operand:SUBDI 0 "register_operand" "=d")
-        (truncate:SUBDI
-	  (lshiftrt:DI (match_operand:DI 1 "register_operand" "d")
-		       (const_int 32))))]
-  "TARGET_64BIT && !TARGET_MIPS16"
-  "dsra\t%0,%1,32"
-  [(set_attr "type" "shift")
-   (set_attr "mode" "<MODE>")])
-
-;; Logical shift by 32 or more results in proper SI values so
-;; truncation is removed by the middle end.
-(define_insn "*<optab>_trunc<mode>_exts"
-  [(set (match_operand:SUBDI 0 "register_operand" "=d")
-        (truncate:SUBDI
-	 (any_shiftrt:DI (match_operand:DI 1 "register_operand" "d")
-			 (match_operand:DI 2 "const_arith_operand" ""))))]
-  "ISA_HAS_EXTS && TARGET_64BIT && UINTVAL (operands[2]) < 32"
-  "exts\t%0,%1,%2,31"
-  [(set_attr "type" "arith")
-   (set_attr "mode" "<MODE>")])

"truncate" is really an extending shift, so from an operational point
of view, the "truncate" section seems just as appropriate as the "shift"
sections.  The reason the patterns are in the truncate section is that
their outputs are smaller than their inputs.  The overall effect of
the patterns is therefore some kind of truncating extraction, with a
plain "truncate" being the special case of a truncating _lowpart_
extraction.  TRULY_NOOP_TRUNCATION makes such operations pretty special,
so I felt it was better to keep them together.  IMO, the fact that we
use *shiftrt codes rather than *_extract codes for truncating highpart
extractions is just a convention detail, so the link to this "truncating
extraction" section is stronger than the link to the shift sections.

It sounds like your new patterns are in the "truncating extraction"
category too, so I'd be happy for them to go in the same section.

I'm not flat-out opposed to your change.  I just wanted to try to explain
the current situation a bit.  Let me know if you're not convinced.

The rest of the patch is OK.

Richard