diff mbox

[i386] PR78509 - TARGET_C_EXCESS_PRECISION should not return "unpredictable" for EXCESS_PRECISION_TYPE_STANDARD

Message ID 1480003228-20257-1-git-send-email-james.greenhalgh@arm.com
State New
Headers show

Commit Message

James Greenhalgh Nov. 24, 2016, 4 p.m. UTC 
Hi,

PR78509 occurs because a target should never return
FLT_EVAL_METHOD_UNPREDICTABLE from TARGET_C_EXCESS_PRECISION when the
TYPE argument to that hook is EXCESS_PRECISION_TYPE_STANDARD or
EXCESS_PRECISION_TYPE_FAST.

When TYPE is one of these two modes, the target is being asked which
explicit excess precision should be applied by tree.c. It doesn't make
sense for this to be "unpredictable". tree.c can only apply predictable
explicit excess precision.

Contrast that with EXCESS_PRECISION_TYPE_IMPLICIT, which can be
unpredictable.

The logic in config/i386/i386.c:i386_excess_precision should be correct
for EXCESS_PRECISION_TYPE_IMPLICIT, but we should not return
FLT_EVAL_METHOD_UNPREDICTABLE for the EXCESS_PRECISION_TYPE_STANDARD case,
for the reasons above. Rather, we want to impose no explicit excess
precision promotions if we are going to be unpredictable anyway - so return
FLT_EVAL_METGHOD_PROMOTE_TO_FLOAT.

By rearranging the logic of the cascade of else if stataments above, we
can make that the default case without duplicating logic.

I've bootstrapped and tested this on x86-64 without any issues, confirmed
that it fixes the previous ICE, and tested the output before my patch set,
and after this patch, with the options suggested by Jakub in the PR:

-fdump-tree-gimple -m32 -msse2 -mno-80387 -fexcess-precision=standard
-fdump-tree-gimple -m32 -msse2 -mfpmath=387+sse -fexcess-precision=standard
-fdump-tree-gimple -m32 -msse2 -mfpmath=387 -fexcess-precision=standard
-fdump-tree-gimple -m32 -msse2 -mfpmath=sse -fexcess-precision=standard
-fdump-tree-gimple -m32 -msse -mno-sse2 -mfpmath=sse -fexcess-precision=standard

With no differences.

I've also updated the documentation to make it clear that returning
FLT_EVAL_METHOD_UNPREDICTABLE is not supported.

OK?

Thanks,
James

---
2016-11-24  James Greenahlgh  <james.greenhalgh@arm.com>

	PR target/78509
	* config/i386/i386.c (i386_excess_precision): Do not return
	FLT_EVAL_METHOD_UNPREDICTABLE when "type" is
	EXCESS_PRECISION_TYPE_STANDARD.
	* target.def (excess_precision): Document that targets should
	not return FLT_EVAL_METHOD_UNPREDICTABLE when "type" is
	EXCESS_PRECISION_TYPE_STANDARD or EXCESS_PRECISION_TYPE_FAST.
	Fix typo in first sentence.
	* doc/tm.texi: Regenerate.

Comments

Uros Bizjak Nov. 25, 2016, 7:26 a.m. UTC | #1 
On Thu, Nov 24, 2016 at 5:00 PM, James Greenhalgh
<james.greenhalgh@arm.com> wrote:
>

> Hi,

>

> PR78509 occurs because a target should never return

> FLT_EVAL_METHOD_UNPREDICTABLE from TARGET_C_EXCESS_PRECISION when the

> TYPE argument to that hook is EXCESS_PRECISION_TYPE_STANDARD or

> EXCESS_PRECISION_TYPE_FAST.

>

> When TYPE is one of these two modes, the target is being asked which

> explicit excess precision should be applied by tree.c. It doesn't make

> sense for this to be "unpredictable". tree.c can only apply predictable

> explicit excess precision.

>

> Contrast that with EXCESS_PRECISION_TYPE_IMPLICIT, which can be

> unpredictable.

>

> The logic in config/i386/i386.c:i386_excess_precision should be correct

> for EXCESS_PRECISION_TYPE_IMPLICIT, but we should not return

> FLT_EVAL_METHOD_UNPREDICTABLE for the EXCESS_PRECISION_TYPE_STANDARD case,

> for the reasons above. Rather, we want to impose no explicit excess

> precision promotions if we are going to be unpredictable anyway - so return

> FLT_EVAL_METGHOD_PROMOTE_TO_FLOAT.

>

> By rearranging the logic of the cascade of else if stataments above, we

> can make that the default case without duplicating logic.

>

> I've bootstrapped and tested this on x86-64 without any issues, confirmed

> that it fixes the previous ICE, and tested the output before my patch set,

> and after this patch, with the options suggested by Jakub in the PR:

>

> -fdump-tree-gimple -m32 -msse2 -mno-80387 -fexcess-precision=standard

> -fdump-tree-gimple -m32 -msse2 -mfpmath=387+sse -fexcess-precision=standard

> -fdump-tree-gimple -m32 -msse2 -mfpmath=387 -fexcess-precision=standard

> -fdump-tree-gimple -m32 -msse2 -mfpmath=sse -fexcess-precision=standard

> -fdump-tree-gimple -m32 -msse -mno-sse2 -mfpmath=sse -fexcess-precision=standard

>

> With no differences.

>

> I've also updated the documentation to make it clear that returning

> FLT_EVAL_METHOD_UNPREDICTABLE is not supported.

>

> OK?

>

> Thanks,

> James

>

> ---

> 2016-11-24  James Greenahlgh  <james.greenhalgh@arm.com>

>

>         PR target/78509

>         * config/i386/i386.c (i386_excess_precision): Do not return

>         FLT_EVAL_METHOD_UNPREDICTABLE when "type" is

>         EXCESS_PRECISION_TYPE_STANDARD.

>         * target.def (excess_precision): Document that targets should

>         not return FLT_EVAL_METHOD_UNPREDICTABLE when "type" is

>         EXCESS_PRECISION_TYPE_STANDARD or EXCESS_PRECISION_TYPE_FAST.

>         Fix typo in first sentence.

>         * doc/tm.texi: Regenerate.


x86 part is OK.

Thanks,
Uros.
Jakub Jelinek Nov. 25, 2016, 7:51 a.m. UTC | #2 
On Fri, Nov 25, 2016 at 08:26:53AM +0100, Uros Bizjak wrote:
> > 2016-11-24  James Greenahlgh  <james.greenhalgh@arm.com>

> >

> >         PR target/78509

> >         * config/i386/i386.c (i386_excess_precision): Do not return

> >         FLT_EVAL_METHOD_UNPREDICTABLE when "type" is

> >         EXCESS_PRECISION_TYPE_STANDARD.

> >         * target.def (excess_precision): Document that targets should

> >         not return FLT_EVAL_METHOD_UNPREDICTABLE when "type" is

> >         EXCESS_PRECISION_TYPE_STANDARD or EXCESS_PRECISION_TYPE_FAST.

> >         Fix typo in first sentence.

> >         * doc/tm.texi: Regenerate.

> 

> x86 part is OK.


The remaining changes are ok as well.

	Jakub
diff mbox

Patch

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 3ccee08..843331b 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -51022,17 +51022,26 @@  ix86_excess_precision (enum excess_precision_type type)
       case EXCESS_PRECISION_TYPE_IMPLICIT:
 	/* Otherwise, the excess precision we want when we are
 	   in a standards compliant mode, and the implicit precision we
-	   provide can be identical.  */
+	   provide would be identical were it not for the unpredictable
+	   cases.  */
 	if (!TARGET_80387)
 	  return FLT_EVAL_METHOD_PROMOTE_TO_FLOAT;
-	else if (TARGET_MIX_SSE_I387)
-	  return FLT_EVAL_METHOD_UNPREDICTABLE;
-	else if (!TARGET_SSE_MATH)
-	  return FLT_EVAL_METHOD_PROMOTE_TO_LONG_DOUBLE;
-	else if (TARGET_SSE2)
-	  return FLT_EVAL_METHOD_PROMOTE_TO_FLOAT;
-	else
-	  return FLT_EVAL_METHOD_UNPREDICTABLE;
+	else if (!TARGET_MIX_SSE_I387)
+	  {
+	    if (!TARGET_SSE_MATH)
+	      return FLT_EVAL_METHOD_PROMOTE_TO_LONG_DOUBLE;
+	    else if (TARGET_SSE2)
+	      return FLT_EVAL_METHOD_PROMOTE_TO_FLOAT;
+	  }
+
+	/* If we are in standards compliant mode, but we know we will
+	   calculate in unpredictable precision, return
+	   FLT_EVAL_METHOD_FLOAT.  There is no reason to introduce explicit
+	   excess precision if the target can't guarantee it will honor
+	   it.  */
+	return (type == EXCESS_PRECISION_TYPE_STANDARD
+		? FLT_EVAL_METHOD_PROMOTE_TO_FLOAT
+		: FLT_EVAL_METHOD_UNPREDICTABLE);
       default:
 	gcc_unreachable ();
     }
diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
index bd82039..ebcadac 100644
--- a/gcc/doc/tm.texi
+++ b/gcc/doc/tm.texi
@@ -948,7 +948,7 @@  Do not define this macro if it would never modify @var{m}.
 @end defmac
 
 @deftypefn {Target Hook} {enum flt_eval_method} TARGET_C_EXCESS_PRECISION (enum excess_precision_type @var{type})
-Return a value, with the same meaning as @code{FLT_EVAL_METHOD} C that describes which excess precision should be applied.  @var{type} is either @code{EXCESS_PRECISION_TYPE_IMPLICIT}, @code{EXCESS_PRECISION_TYPE_FAST}, or @code{EXCESS_PRECISION_TYPE_STANDARD}.  For @code{EXCESS_PRECISION_TYPE_IMPLICIT}, the target should return which precision and range operations will be implictly evaluated in regardless of the excess precision explicitly added.  For @code{EXCESS_PRECISION_TYPE_STANDARD} and @code{EXCESS_PRECISION_TYPE_FAST}, the target should return the explicit excess precision that should be added depending on the value set for @option{-fexcess-precision=@r{[}standard@r{|}fast@r{]}}.
+Return a value, with the same meaning as the C99 macro @code{FLT_EVAL_METHOD} that describes which excess precision should be applied.  @var{type} is either @code{EXCESS_PRECISION_TYPE_IMPLICIT}, @code{EXCESS_PRECISION_TYPE_FAST}, or @code{EXCESS_PRECISION_TYPE_STANDARD}.  For @code{EXCESS_PRECISION_TYPE_IMPLICIT}, the target should return which precision and range operations will be implictly evaluated in regardless of the excess precision explicitly added.  For @code{EXCESS_PRECISION_TYPE_STANDARD} and @code{EXCESS_PRECISION_TYPE_FAST}, the target should return the explicit excess precision that should be added depending on the value set for @option{-fexcess-precision=@r{[}standard@r{|}fast@r{]}}. Note that unpredictable explicit excess precision does not make sense, so a target should never return @code{FLT_EVAL_METHOD_UNPREDICTABLE} when @var{type} is @code{EXCESS_PRECISION_TYPE_STANDARD} or @code{EXCESS_PRECISION_TYPE_FAST}.
 @end deftypefn
 
 @deftypefn {Target Hook} machine_mode TARGET_PROMOTE_FUNCTION_MODE (const_tree @var{type}, machine_mode @var{mode}, int *@var{punsignedp}, const_tree @var{funtype}, int @var{for_return})
diff --git a/gcc/target.def b/gcc/target.def
index efcc336..85a0ac0 100644
--- a/gcc/target.def
+++ b/gcc/target.def
@@ -5466,9 +5466,9 @@  DEFHOOK_UNDOC
 
 DEFHOOK
 (excess_precision,
- "Return a value, with the same meaning as @code{FLT_EVAL_METHOD} C that\
- describes which excess precision should be applied.  @var{type} is\
- either @code{EXCESS_PRECISION_TYPE_IMPLICIT},\
+ "Return a value, with the same meaning as the C99 macro\
+ @code{FLT_EVAL_METHOD} that describes which excess precision should be\
+ applied.  @var{type} is either @code{EXCESS_PRECISION_TYPE_IMPLICIT},\
  @code{EXCESS_PRECISION_TYPE_FAST}, or\
  @code{EXCESS_PRECISION_TYPE_STANDARD}.  For\
  @code{EXCESS_PRECISION_TYPE_IMPLICIT}, the target should return which\
@@ -5477,7 +5477,11 @@  DEFHOOK
  @code{EXCESS_PRECISION_TYPE_STANDARD} and\
  @code{EXCESS_PRECISION_TYPE_FAST}, the target should return the\
  explicit excess precision that should be added depending on the\
- value set for @option{-fexcess-precision=@r{[}standard@r{|}fast@r{]}}.",
+ value set for @option{-fexcess-precision=@r{[}standard@r{|}fast@r{]}}.\
+ Note that unpredictable explicit excess precision does not make sense,\
+ so a target should never return @code{FLT_EVAL_METHOD_UNPREDICTABLE}\
+ when @var{type} is @code{EXCESS_PRECISION_TYPE_STANDARD} or\
+ @code{EXCESS_PRECISION_TYPE_FAST}.",
  enum flt_eval_method, (enum excess_precision_type type),
  default_excess_precision)