On 11/1/18 1:15 PM, Jakub Jelinek wrote:
> On Thu, Nov 01, 2018 at 01:09:16PM +0100, Martin Liška wrote:
>> -range 0.0 to 1.0, inclusive.
>> +range 0.0 to 1.0, inclusive. The @var{probability} argument must be
>> +a compiler time constant.
>
> When you say must, I think error_at should be used rather than warning_at.
> If others disagree I'm open for leaving it as is.
Error is fine for me as well.
>
>> @@ -2474,6 +2481,11 @@ expr_expected_value_1 (tree type, tree op0, enum
>> tree_code code,
>> *predictor = PRED_BUILTIN_EXPECT_WITH_PROBABILITY;
>> *probability = probi;
>> }
>> + else
>> + warning_at (gimple_location (def), 0,
>> + "probability argument %qE must be a in the "
>> + "range 0.0 to 1.0", prob);
>
> Wrong indentation.
>
> And, no diagnostics for -O0 (which should also be covered by a testcase).
Test for that added.
>
>> +/* { dg-options "-O2 -fdump-tree-profile_estimate -frounding-math" } */
>
> Why the -frounding-math options?
I remember I had some issue with:
tree r = fold_build2_initializer_loc (UNKNOWN_LOCATION,
MULT_EXPR, t, prob,
base);
on targets with a non-IEEE floating point arithmetics (s390?).
I think test
> coverage should handle both that and when that option is not used
> if that option makes any difference.
It will eventually pop up if we install new tests w/o rounding math.
>
> Jakub
>
Martin
>From 7e0834a2ebe1a3fb83304197a843dca63332bc78 Mon Sep 17 00:00:00 2001
From: marxin <mli...@suse.cz>
Date: Tue, 30 Oct 2018 14:01:59 +0100
Subject: [PATCH] Verify that last argument of
__builtin_expect_with_probability is a real cst (PR c/87811).
gcc/ChangeLog:
2018-10-30 Martin Liska <mli...@suse.cz>
PR c/87811
* predict.c (expr_expected_value_1): Verify
that last argument is a real constants and emit
error.
gcc/testsuite/ChangeLog:
2018-10-30 Martin Liska <mli...@suse.cz>
PR c/87811
* gcc.dg/pr87811.c: New test.
* gcc.dg/pr87811-2.c: Likewise.
* gcc.dg/pr87811-3.c: Likewise.
gcc/ChangeLog:
2018-10-30 Martin Liska <mli...@suse.cz>
* doc/extend.texi: Update constrain about the last argument
of __builtin_expect_with_probability.
---
gcc/doc/extend.texi | 3 ++-
gcc/predict.c | 12 ++++++++++++
gcc/testsuite/gcc.dg/pr87811-2.c | 13 +++++++++++++
gcc/testsuite/gcc.dg/pr87811-3.c | 11 +++++++++++
gcc/testsuite/gcc.dg/pr87811.c | 13 +++++++++++++
5 files changed, 51 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/gcc.dg/pr87811-2.c
create mode 100644 gcc/testsuite/gcc.dg/pr87811-3.c
create mode 100644 gcc/testsuite/gcc.dg/pr87811.c
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index e2b9ee11a54..3dabd486dbf 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -12046,7 +12046,8 @@ when testing pointer or floating-point values.
This function has the same semantics as @code{__builtin_expect},
but the caller provides the expected probability that @var{exp} == @var{c}.
The last argument, @var{probability}, is a floating-point value in the
-range 0.0 to 1.0, inclusive.
+range 0.0 to 1.0, inclusive. The @var{probability} argument must be
+a compiler time constant.
@end deftypefn
@deftypefn {Built-in Function} void __builtin_trap (void)
diff --git a/gcc/predict.c b/gcc/predict.c
index ab2dc8ed031..80a8d6846f7 100644
--- a/gcc/predict.c
+++ b/gcc/predict.c
@@ -2467,6 +2467,13 @@ expr_expected_value_1 (tree type, tree op0, enum tree_code code,
base = build_real_from_int_cst (t, base);
tree r = fold_build2_initializer_loc (UNKNOWN_LOCATION,
MULT_EXPR, t, prob, base);
+ if (TREE_CODE (r) != REAL_CST)
+ {
+ error_at (gimple_location (def),
+ "probability argument %qE must be a compile "
+ "time constant", prob);
+ return NULL;
+ }
HOST_WIDE_INT probi
= real_to_integer (TREE_REAL_CST_PTR (r));
if (probi >= 0 && probi <= REG_BR_PROB_BASE)
@@ -2474,6 +2481,11 @@ expr_expected_value_1 (tree type, tree op0, enum tree_code code,
*predictor = PRED_BUILTIN_EXPECT_WITH_PROBABILITY;
*probability = probi;
}
+ else
+ error_at (gimple_location (def),
+ "probability argument %qE must be a in the "
+ "range 0.0 to 1.0", prob);
+
return gimple_call_arg (def, 1);
}
diff --git a/gcc/testsuite/gcc.dg/pr87811-2.c b/gcc/testsuite/gcc.dg/pr87811-2.c
new file mode 100644
index 00000000000..aa30ddff47b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr87811-2.c
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-profile_estimate" } */
+
+void bar (void);
+
+void
+foo (int i)
+{
+ if (__builtin_expect_with_probability (i, 0, 2.0f)) /* { dg-error "probability argument .* must be a in the range 0\\\.0 to 1\\\.0" } */
+ bar ();
+}
+
+/* { dg-final { scan-tree-dump-not "__builtin_expect_with_probability heuristics of edge" "profile_estimate"} } */
diff --git a/gcc/testsuite/gcc.dg/pr87811-3.c b/gcc/testsuite/gcc.dg/pr87811-3.c
new file mode 100644
index 00000000000..720d154a8d4
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr87811-3.c
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-options "-fdump-tree-profile_estimate" } */
+
+void bar (void);
+
+void
+foo (int i)
+{
+ if (__builtin_expect_with_probability (i, 0, 2.0f))
+ bar ();
+}
diff --git a/gcc/testsuite/gcc.dg/pr87811.c b/gcc/testsuite/gcc.dg/pr87811.c
new file mode 100644
index 00000000000..9045c8ea957
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr87811.c
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-profile_estimate" } */
+
+void bar (void);
+
+void
+foo (int i, double d)
+{
+ if (__builtin_expect_with_probability (i, 0, d)) /* { dg-error "probability argument .d. must be a compile time constant" } */
+ bar ();
+}
+
+/* { dg-final { scan-tree-dump-not "__builtin_expect_with_probability heuristics of edge" "profile_estimate"} } */
--
2.19.0
