On 11/1/18 7:45 AM, Martin Liška wrote:
> 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
>
>
> 0001-Verify-that-last-argument-of-__builtin_expect_with_p.patch
>
> 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
OK
jeff
