This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH, i386] Unify TARGET_SSE_MATH and TARGET_MIX_SSE_I387 in insn constraints
- From: Richard Henderson <rth at redhat dot com>
- To: Uros Bizjak <uros at kss-loka dot si>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Sun, 12 Dec 2004 22:53:05 -0800
- Subject: Re: [PATCH, i386] Unify TARGET_SSE_MATH and TARGET_MIX_SSE_I387 in insn constraints
- References: <41B72A01.4080702@kss-loka.si>
Having just run into another sse math problem, I changed my mind
about this patch for 4.0. ;-)
On Wed, Dec 08, 2004 at 05:21:21PM +0100, Uros Bizjak wrote:
> (define_expand "floathidf2"
> [(set (match_operand:DF 0 "register_operand" "")
> (float:DF (match_operand:HI 1 "nonimmediate_operand" "")))]
> - "TARGET_SSE2 || TARGET_80387"
> + "TARGET_80387 || (TARGET_SSE2 && TARGET_SSE_MATH)"
> {
> - if (TARGET_SSE && TARGET_SSE_MATH)
> + if (TARGET_SSE_MATH)
This looks buggy. Given TARGET_SSE_MATH, but only sse1 enabled,
we'll not follow the TARGET_80387 path.
I would guess this situation is common enough with doubles to
warrent the creation of a new TARGET_SSE2_MATH macro to minimize
mistakes.
> -(define_insn "sqrtsf2_1_sse_only"
> +(define_insn "*sqrtsf2_sse"
> [(set (match_operand:SF 0 "register_operand" "=x")
> (sqrt:SF (match_operand:SF 1 "nonimmediate_operand" "xm")))]
> - "TARGET_SSE_MATH && (!TARGET_80387 || !TARGET_MIX_SSE_I387)"
> + "TARGET_SSE_MATH && !TARGET_MIX_SSE_I387"
> "sqrtss\t{%1, %0|%0, %1}"
> [(set_attr "type" "sse")
> (set_attr "mode" "SF")
> (set_attr "athlon_decode" "*")])
...
> +(define_insn "*sqrtsf2_mixed"
> + [(set (match_operand:SF 0 "register_operand" "=f#x,x#f")
> + (sqrt:SF (match_operand:SF 1 "nonimmediate_operand" "0#x,xm#f")))]
> + "TARGET_USE_FANCY_MATH_387 && TARGET_MIX_SSE_I387"
> + "@
> + fsqrt
> + sqrtss\t{%1, %0|%0, %1}"
> + [(set_attr "type" "fpspc,sse")
> + (set_attr "mode" "SF,SF")
> + (set_attr "athlon_decode" "direct,*")])
Seems like the sqrtsf2_sse condition should be
TARGET_SSE_MATH && (!TARGET_MIX_SSE_I387 || !TARGET_USE_FANCY_MATH_387)
so that we use the sse builtin even when 387 fancies are disabled.
Alternately, we're canonicalizing these patterns into the wrong order.
Perhaps the order should be
(define_insn "*foo_mixed"
[...]
"TARGET_MIX_SSE_I387"
...)
(define_insn "*foo_sse"
[...]
"TARGET_SSE_MATH"
...)
(define_insn "*foo_i387"
[...]
"TARGET_80387"
...)
(define_insn "*fancy_mixed"
[...]
"TARGET_USE_FANCY_MATH_387 && TARGET_MIX_SSE_I387"
...)
(define_insn "*fancy_sse"
[...]
"TARGET_SSE_MATH"
...)
(define_insn "*fancy_i387"
[...]
"TARGET_USE_FANCY_MATH_387"
...)
In other words, explicitly make use of the first-pattern-matches rule.
Which is nice, because the individual conditions are smaller.
r~