Re: [PATCH, MIPS] Add exts Octeon instruction

[Richard Sandiford <rdsandiford at googlemail dot com>]

Adam Nemet <anemet@caviumnetworks.com> writes:
> octeon-exts-1.c and octeon-exts-2.c are the testcases for the actual
> bitfield extraction.  There is still more work in this area as for
> example we generate really bad code for octeon-exts-1.c with
> optimization.  (The test is written without optimization.)  This is due
> to the tree optimizers.  I will look at this later.

Yeah, SRA can make things much worse sometimes.

> 	* config/mips/mips.h (ISA_HAS_EXTS): New macro.
> 	* config/mips/mips.md (*ashr_trunc<mode>): Name the pattern
> 	combining an arithmetic right shift by more than 31 and a
> 	trunction.  Don't match for out-of-range shift amounts.  Set
> 	attribute mode to <MODE>.
> 	(*lshr32_trunc<mode>): Name the pattern combining a logical right
> 	shift by 32 and and a truncation.  Set attribute mode to <MODE>.
> 	(*<optab>_trunc<mode>_exts): New pattern for truncated right
> 	shifts by less than 32.
> 	(extv): Change predicate on first operand to accept registers.
> 	Change predicate of the other operands from immediate_operand to
> 	const_int_operand.  Expand exts when source is a register.
> 	(extzv): Change predicate of the constant operands from
> 	immediate_operand to const_int_operand.
> 	(extzv<mode>): Change predicate of the constant operands from
> 	immediate_operand to const_int_operand and no constraint. Also
> 	remove mode.
> 	(*extzv_trunc<mode>_exts): New pattern.
>
> testsuite/
> 	* gcc.target/mips/truncate-2.c: New test.
> 	* gcc.target/mips/octeon-exts-1.c: New test.
> 	* gcc.target/mips/octeon-exts-2.c: New test.
> 	* gcc.target/mips/octeon-exts-3.c: New test.
> 	* gcc.target/mips/octeon-exts-4.c: New test.

OK, but...

> +;; Logical shift by more than 32 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>")])

...it might be better to say "32 or more" instead of "more than 32" here,
to explain why this pattern uses "<" while other exts patterns use "<=".

The tests look nicely thorough, thanks.

Richard