Re: [PATCH 36/42] i386: Correct <ssse3_avx2>_pmulhrsw<mode>3[_mask]

[Uros Bizjak <ubizjak@gmail.com>]

On 2/16/19, H.J. Lu <hjl.tools@gmail.com> wrote:
> There is no V4HI pmulhrsw in AVX512BW and V4HI/V8HI pmulhrsw don't require
> AVX2.  To support TARGET_MMX_WITH_SSE, replace nonimmediate_operand with
> register_pmulhrswmem_operand in <ssse3_avx2>_pmulhrsw<mode>3.
>
> 	PR target/89372
> 	* config/i386/predicates.md (register_pmulhrswmem_operand): New.
> 	* config/i386/sse.md (PMULHRSW): Remove V4HI.
> 	(PMULHRSW_MMX): New.
> 	(<ssse3_avx2>_pmulhrsw<mode>3): Replace PMULHRSW with
> 	PMULHRSW_MMX.  Require TARGET_SSSE3, not TARGET_AVX2.  Replace
> 	nonimmediate_operand with register_pmulhrswmem_operand.

Complications like the above usually points to wrong macroization
choice. Please try to split out V4HImode pattern.

Also, please separate the fix to a separate patch, which should be
committed independently as a fix before gcc-9 is released.

Uros.

> ---
>  gcc/config/i386/predicates.md |  7 +++++++
>  gcc/config/i386/sse.md        | 15 +++++++++------
>  2 files changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/gcc/config/i386/predicates.md b/gcc/config/i386/predicates.md
> index f3c2f72de54..b7cb26a81fe 100644
> --- a/gcc/config/i386/predicates.md
> +++ b/gcc/config/i386/predicates.md
> @@ -56,6 +56,13 @@
>         (and (not (match_test "TARGET_MMX_WITH_SSE"))
>  	    (match_operand 0 "memory_operand"))))
>
> +;; Match register operands, but include memory operands for
> +;; !(TARGET_MMX_WITH_SSE && mode == V4HImode).
> +(define_predicate "register_pmulhrswmem_operand"
> +  (ior (match_operand 0 "register_operand")
> +       (and (not (match_test "TARGET_MMX_WITH_SSE && mode == V4HImode"))
> +	    (match_operand 0 "memory_operand"))))
> +
>  ;; True if the operand is an SSE register.
>  (define_predicate "sse_reg_operand"
>    (and (match_code "reg")
> diff --git a/gcc/config/i386/sse.md b/gcc/config/i386/sse.md
> index 92f5ad17156..379da16615d 100644
> --- a/gcc/config/i386/sse.md
> +++ b/gcc/config/i386/sse.md
> @@ -15579,7 +15579,7 @@
>     (set_attr "mode" "DI,TI,TI")])
>
>  (define_mode_iterator PMULHRSW
> -  [V4HI V8HI (V16HI "TARGET_AVX2")])
> +  [V8HI (V16HI "TARGET_AVX2")])
>
>  (define_expand "<ssse3_avx2>_pmulhrsw<mode>3_mask"
>    [(set (match_operand:PMULHRSW 0 "register_operand")
> @@ -15604,21 +15604,24 @@
>    ix86_fixup_binary_operands_no_copy (MULT, <MODE>mode, operands);
>  })
>
> +(define_mode_iterator PMULHRSW_MMX
> +  [V4HI V8HI (V16HI "TARGET_AVX2")])
> +
>  (define_expand "<ssse3_avx2>_pmulhrsw<mode>3"
> -  [(set (match_operand:PMULHRSW 0 "register_operand")
> -	(truncate:PMULHRSW
> +  [(set (match_operand:PMULHRSW_MMX 0 "register_operand")
> +	(truncate:PMULHRSW_MMX
>  	  (lshiftrt:<ssedoublemode>
>  	    (plus:<ssedoublemode>
>  	      (lshiftrt:<ssedoublemode>
>  		(mult:<ssedoublemode>
>  		  (sign_extend:<ssedoublemode>
> -		    (match_operand:PMULHRSW 1 "nonimmediate_operand"))
> +		    (match_operand:PMULHRSW_MMX 1 "register_pmulhrswmem_operand"))
>  		  (sign_extend:<ssedoublemode>
> -		    (match_operand:PMULHRSW 2 "nonimmediate_operand")))
> +		    (match_operand:PMULHRSW_MMX 2 "register_pmulhrswmem_operand")))
>  		(const_int 14))
>  	      (match_dup 3))
>  	    (const_int 1))))]
> -  "TARGET_AVX2"
> +  "TARGET_SSSE3"
>  {
>    operands[3] = CONST1_RTX(<MODE>mode);
>    ix86_fixup_binary_operands_no_copy (MULT, <MODE>mode, operands);
> --
> 2.20.1
>
>