On 10/31/18 11:17 AM, Jakub Jelinek wrote:
> On Wed, Oct 31, 2018 at 11:04:32AM +0100, Martin Liška wrote:
>> Hi.
>>
>> As Jakub pointed out we should not ICE when last argument
>> of __builtin_expect_with_probability is not a real cst.
>> Plus I documented the behavior.
>
> That is not what you've implemented. The documentation says that
> it must be a compile time constant, but nothing enforces it, just silently
> ignores it. So, either diagnose it at the time when
> __builtin_expect_with_probability is removed from the IL, i.e.
> the code you've touched? and expand_builtin_expect_with_probability
> and the argument would be really a late constant (constant after
> inlining and optimizations), or diagnose it in fold_builtin_expect
> and replace with the argument at that point. And, please diagnose also the
> cases where the probability is not >= 0.0 and <= 1.0.
I provided 2 warnings for both of these situations.
>> * doc/extend.texi: Update constrain about the last arguemnt
>
> s/arguemnt/argument/
Fixed.
New patch survives regression tests on x86_64-linux-gnu.
Martin
>
> Jakub
>
>From bae9e3abeeea9b4e2dcde8c1581efaaffd800899 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.
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/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.c | 13 +++++++++++++
4 files changed, 40 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/gcc.dg/pr87811-2.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..43621653fee 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)
+ {
+ warning_at (gimple_location (def), 0,
+ "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
+ warning_at (gimple_location (def), 0,
+ "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..27d910c9305
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr87811-2.c
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-profile_estimate -frounding-math" } */
+
+void bar (void);
+
+void
+foo (int i)
+{
+ if (__builtin_expect_with_probability (i, 0, 2.0f)) /* { dg-warning "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.c b/gcc/testsuite/gcc.dg/pr87811.c
new file mode 100644
index 00000000000..18e5c0d97db
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr87811.c
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-profile_estimate -frounding-math" } */
+
+void bar (void);
+
+void
+foo (int i, double d)
+{
+ if (__builtin_expect_with_probability (i, 0, d)) /* { dg-warning "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
