On Tue, Mar 13, 2018 at 9:30 PM, Jakub Jelinek <ja...@redhat.com> wrote:
> Hi!
>
> As mentioned in bugzilla, when e.g. sel-sched queries (indirectly) before
> reload
> some attributes like get_attr_type that depend on alternatives, GCC attempts
> to constrain the operands in non-strict mode, which implies that if
> reg_class_for_constraint doesn't return NO_REGS, it is ok, otherwise the
> constraint needs to match (the actual code is more complex of course).
> The *float<SWI48:mode><MODEF:mode>2_mixed pattern has different type
> attributes between different alternatives, uses nonimmediate_operand for the
> input and uses "m" constraint for it in all but one alternative; in that
> alternative it has "r" constraint for the input and "Yc" for output, which
> depending on tuning is either same as "v" or NO_REGS. So, on those tunings
> even in non-strict mode, if the input is a REG we fail to constrain the insn
> and ICE.
>
> The following patch fixes it by reverting the offending patch (as asked in
> the PR), even with the patch reverted the reported issue doesn't reproduce
> and in theory there is nothing wrong on emitting direct conversions even in
> these tunings in cold blocks, the hw supports it, just it is slow, but also
> smaller.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> As mentioned in the PR, another alternative that works is adding another
> alternative next to that Yc <- r, e.g. !???*v <- r, which will allow the
> pre-reload attribute queries, but will very likely not be used otherwise.
>
> 2018-03-13 Jakub Jelinek <ja...@redhat.com>
>
> PR target/84844
> Revert
> 2017-04-20 Uros Bizjak <ubiz...@gmail.com>
>
> PR target/78090
> * config/i386/constraints.md (Yc): New register constraint.
> * config/i386/i386.md (*float<SWI48:mode><MODEF:mode>2_mixed):
> Use Yc constraint for alternative 2 of operand 0. Remove
> preferred_for_speed attribute.
>
> * gcc.target/i386/pr84844.c: New test.
OK.
Perhaps some time in future, we should change all these inter-unit
constraints to use preferred_for_speed attribute. As with the attached
patch, these insn are not invalid instructions, so we can emit them in
certain cases (-Os), even for AMD targets. Conditional register
constraints made sense were
--- gcc/config/i386/constraints.md.jj 2018-02-26 20:49:57.299331387 +0100
> +++ gcc/config/i386/constraints.md 2018-03-13 13:47:22.285093035 +0100
> @@ -99,7 +99,6 @@ (define_register_constraint "w" "TARGET_
>
> ;; We use the Y prefix to denote any number of conditional register sets:
> ;; z First SSE register.
> -;; c SSE inter-unit conversions enabled
> ;; i SSE2 inter-unit moves to SSE register enabled
> ;; j SSE2 inter-unit moves from SSE register enabled
> ;; d any EVEX encodable SSE register for AVX512BW target or any SSE
> register
> @@ -124,10 +123,6 @@ (define_register_constraint "w" "TARGET_
> (define_register_constraint "Yz" "TARGET_SSE ? SSE_FIRST_REG : NO_REGS"
> "First SSE register (@code{%xmm0}).")
>
> -(define_register_constraint "Yc"
> - "TARGET_SSE && TARGET_INTER_UNIT_CONVERSIONS ? ALL_SSE_REGS : NO_REGS"
> - "@internal Any SSE register, when SSE and inter-unit conversions are
> enabled.")
> -
> (define_register_constraint "Yi"
> "TARGET_SSE2 && TARGET_INTER_UNIT_MOVES_TO_VEC ? ALL_SSE_REGS : NO_REGS"
> "@internal Any SSE register, when SSE2 and inter-unit moves to vector
> registers are enabled.")
> --- gcc/config/i386/i386.md.jj 2018-03-13 13:40:44.082903460 +0100
> +++ gcc/config/i386/i386.md 2018-03-13 13:47:22.284093034 +0100
> @@ -5325,7 +5325,7 @@ (define_expand "float<SWI48:mode><MODEF:
> })
>
> (define_insn "*float<SWI48:mode><MODEF:mode>2_mixed"
> - [(set (match_operand:MODEF 0 "register_operand" "=f,Yc,v")
> + [(set (match_operand:MODEF 0 "register_operand" "=f,v,v")
> (float:MODEF
> (match_operand:SWI48 1 "nonimmediate_operand" "m,r,m")))]
> "SSE_FLOAT_MODE_P (<MODEF:MODE>mode) && TARGET_SSE_MATH"
> @@ -5354,6 +5354,10 @@ (define_insn "*float<SWI48:mode><MODEF:m
> && X87_ENABLE_FLOAT (<MODEF:MODE>mode,
> <SWI48:MODE>mode)")
> ]
> + (symbol_ref "true")))
> + (set (attr "preferred_for_speed")
> + (cond [(eq_attr "alternative" "1")
> + (symbol_ref "TARGET_INTER_UNIT_CONVERSIONS")]
> (symbol_ref "true")))])
>
> (define_insn "*float<SWI48x:mode><MODEF:mode>2_i387"
> --- gcc/testsuite/gcc.target/i386/pr84844.c.jj 2018-03-13 13:12:50.569130703
> +0100
> +++ gcc/testsuite/gcc.target/i386/pr84844.c 2018-03-13 12:21:04.553643164
> +0100
> @@ -0,0 +1,10 @@
> +/* PR target/84844 */
> +/* { dg-do compile } */
> +/* { dg-options "-march=bdver1 -O2 -fschedule-insns -fselective-scheduling"
> } */
> +
> +double
> +foo (int *x, int y, int z)
> +{
> + *x = y;
> + return z;
> +}
>
> Jakub
