Re: libgo patch committed: Fix signal counting for glibc 2.26

[Richard Biener]

On Thu, Aug 3, 2017 at 8:11 PM, Ian Lance Taylor  wrote:
> This patch to libgo changes the mksigtab script to recognize the glibc
> 2.26 NSIG expression.  Bootstrapped and ran Go testsuite on
> x86_64-pc-linux-gnu.  Committed to mainline.  Will commit to GCC 7
> branch when it reopens.  This fixes GCC PR 81617 and
> https://golang.org/issue/21147.

The patch looks pretty safe as it only changes things when nsig is empty
so can you install it on the branch now so that 7.2 will build Go fine with
glibc 2.26?

Thanks,
Richard.

>
> Ian

[PATCH] Fold (int *)&a + 4 to a[1] using offset_int (PR middle-end/81695)

[Marek Polacek]

We were crashing because size_binop_loc got operands of different types:
sizetype and ssizetype.  Richi suggested performing the computation in
offset_int which my patch tries to do.  Unsure about the sdiv_trunc part,
what do I use instead of EXACT_DIV_EXPR?

Bootstrapped/regtested on x86_64-linux, ok for trunk?  Testing on ppc64le
in progress.

2017-08-04  Marek Polacek  

PR middle-end/81695
* fold-const.c (fold_indirect_ref_1): For ((int *)&a + 4 -> a[1],
perform the computation in offset_int.

* gcc.dg/pr81695.c: New test.

diff --git gcc/fold-const.c gcc/fold-const.c
index ed6c289a64b..64a84eb0197 100644
--- gcc/fold-const.c
+++ gcc/fold-const.c
@@ -14106,14 +14106,17 @@ fold_indirect_ref_1 (location_t loc, tree type, tree 
op0)
   && type == TREE_TYPE (op00type))
{
  tree type_domain = TYPE_DOMAIN (op00type);
- tree min_val = size_zero_node;
- if (type_domain && TYPE_MIN_VALUE (type_domain))
-   min_val = TYPE_MIN_VALUE (type_domain);
- op01 = size_binop_loc (loc, EXACT_DIV_EXPR, op01,
-TYPE_SIZE_UNIT (type));
- op01 = size_binop_loc (loc, PLUS_EXPR, op01, min_val);
- return build4_loc (loc, ARRAY_REF, type, op00, op01,
-NULL_TREE, NULL_TREE);
+ tree min = TYPE_MIN_VALUE (type_domain);
+ if (min && TREE_CODE (min) == INTEGER_CST)
+   {
+ offset_int off = wi::to_offset (op01);
+ offset_int el_sz = wi::to_offset (TYPE_SIZE_UNIT (type));
+ off = wi::sdiv_trunc (off, el_sz);
+ off = off + wi::to_offset (min);
+ op01 = wide_int_to_tree (sizetype, off);
+ return build4_loc (loc, ARRAY_REF, type, op00, op01,
+NULL_TREE, NULL_TREE);
+   }
}
}
 }
diff --git gcc/testsuite/gcc.dg/pr81695.c gcc/testsuite/gcc.dg/pr81695.c
index e69de29bb2d..c3452580f1c 100644
--- gcc/testsuite/gcc.dg/pr81695.c
+++ gcc/testsuite/gcc.dg/pr81695.c
@@ -0,0 +1,11 @@
+/* PR middle-end/81695 */
+/* { dg-do compile } */
+/* { dg-options "" } */
+
+int z[] = { };
+
+int
+main (void)
+{
+  __builtin_printf ("%d\n", *(z + 1));
+}

Marek

Re: [PING^4][PATCH v2] Generate reproducible output independently of the build-path

[Yury Gribov]

On Thu, Aug 3, 2017 at 12:45 PM, Ximin Luo  wrote:
> Yury Gribov:
>> On 03.08.2017 3:06, Ximin Luo wrote:
>>> Jeff Law:
 On 07/21/2017 10:15 AM, Ximin Luo wrote:
> (Please keep me on CC, I am not subscribed)
>
>
> Proposal
> 
>
> This patch series adds a new environment variable BUILD_PATH_PREFIX_MAP. 
> When
> this is set, GCC will treat this as extra implicit 
> "-fdebug-prefix-map=$value"
> command-line arguments that precede any explicit ones. This makes the 
> final
> binary output reproducible, and also hides the unreproducible value (the 
> source
> path prefixes) from CFLAGS et. al. which many build tools (understandably)
> embed as-is into their build output.
 I'd *really* avoid doing this with magic environment variables.  Make it
 a first class option to the compiler.  Yes, it means projects that want
 this behavior have to arrange to pass that flag to their compiler, but
 IMHO that's much preferred over environment variables.

 Jeff

>>>
>>> Hi Jeff,
>>>
>>> If by "first class option" you meant a command-line flag, GCC *already has* 
>>> that (-fdebug-prefix-map) > and it wasn't enough to achieve reproducibility 
>>> in many cases we tested.
>>
>> Shouldn't -fdebug-prefix-map be updated to use the same syntax as 
>> BUILD_PATH_PREFIX_MAP?
>>
>
> -fdebug-prefix-map is a CLI option and can be given multiple times, each flag 
> given is in the form of $from=$to where $from can't contain a '='.
>
> BUILD_PATH_PREFIX_MAP is a single envvar that encodes a list-of-pairs of the 
> form $to=$from:$to=$from with some escaping for flexibility and to support 
> things like windows paths. Since it's a new envvar, Ian Jackson suggested 
> $to=$from to emphasise the reproducible ($to) part. I liked the idea so I 
> implemented it like that. (We did a lot of bikeshedding over on the 
> rb-general mailing list about the exact format and this is what we settled 
> on, I'd like to avoid getting into that again but would nevertheless do it, 
> if it's necessary to get this patch accepted.)
>
> Because -fdebug-prefix-map currently only encodes one $from=$to pair, it 
> would be a very disruptive and highly backward-incompatible change to make it 
> use the same syntax as B_P_P_M. A slightly less disruptive but still 
> backward-incompatible change would be to make it encode a single $to=$from 
> pair, but I don't really see the advantage to doing so - what were your 
> thoughts on this?

I believe it would much easier to reason about environment variable
behavior when it boils down to "prepend some standard flag to
command-line flags".  It would also simplify maintenance of local
compiler patch as core functionality can be merged to mainline GCC
whereas debatable environment variable part stays in the distro.

> If by "first class option" you meant a command-line flag, GCC *already has* 
> that (-fdebug-prefix-map) and it wasn't enough to achieve reproducibility in 
> many cases we tested.
> dpkg-buildflags actually already adds these flags to CFLAGS CXXFLAGS etc on 
> Debian. However, with this patch using the environment variable, we are able 
> to reproduce 1800 more packages out of 26000.

Just curious, why -fdebug-prefix-map (maybe modified to support
multiple renames) was not enough for these packages (and why they
can't be fixed instead)?

-Y

Re: Handle data dependence relations with different bases

[Richard Biener]

On Thu, Jul 27, 2017 at 2:19 PM, Richard Sandiford
 wrote:
> Richard Sandiford  writes:
>> Eric Botcazou  writes:
>>> [Sorry for missing the previous messages]
>>>
 Thanks.  Just been retesting, and I think I must have forgotten
 to include Ada last time.  It turns out that the patch causes a dg-scan
 regression in gnat.dg/vect17.adb, because we now think that if the
 array RECORD_TYPEs *do* alias in:

procedure Add (X, Y : aliased Sarray; R : aliased out Sarray) is
begin
   for I in Sarray'Range loop
  R(I) := X(I) + Y(I);
   end loop;
end;

 then the dependence distance must be zero.  Eric, does that hold true
 for Ada?  I.e. if X and R (or Y and R) alias, must it be the case that
 X(I) can only alias R(I) and not for example R(I-1) or R(I+1)?
>>>
>>> Yes, I'd think so (even without the artificial RECORD_TYPE around the 
>>> arrays).
>>
>> Good!
>>
 2017-06-07  Richard Sandiford  

 gcc/testsuite/
 * gnat.dg/vect17.ads (Sarray): Increase range to 1 .. 5.
 * gnat.dg/vect17.adb (Add): Create a dependence distance of 1
 when X = R or Y = R.
>>>
>>> I think that you need to modify vect15 and vect16 the same way.
>>
>> Ah, yeah.  And doing that shows that I'd not handled safelen for
>> DDR_COULD_BE_INDEPENDENT_P.  I've fixed that locally.
>>
>> How does this look?  Tested on x86_64-linux-gnu both without the
>> vectoriser changes and with the fixed vectoriser patch.
>
> Here's a version of the patch that handles safelen.  I split the
> handling out into a new function (vect_analyze_possibly_independent_ddr)
> since it was getting too big to do inline.
>
> Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?

Ok.

Did you check whether BB vectorization is affected?  See
vect_slp_analyze_instance_dependence
and friends.  It's quite conservative but given the prefetching change
I wonder if we need
to rule out DDR_COULD_BE_INDEPENDENT_P?

Thanks,
Richard.

> Thanks,
> Richard
>
>
> 2017-07-27  Richard Sandiford  
>
> gcc/
> * tree-data-ref.h (subscript): Add access_fn field.
> (data_dependence_relation): Add could_be_independent_p.
> (SUB_ACCESS_FN, DDR_COULD_BE_INDEPENDENT_P): New macros.
> (same_access_functions): Move to tree-data-ref.c.
> * tree-data-ref.c (ref_contains_union_access_p): New function.
> (access_fn_component_p): Likewise.
> (access_fn_components_comparable_p): Likewise.
> (dr_analyze_indices): Add a reference to access_fn_component_p.
> (dump_data_dependence_relation): Use SUB_ACCESS_FN instead of
> DR_ACCESS_FN.
> (constant_access_functions): Likewise.
> (add_other_self_distances): Likewise.
> (same_access_functions): Likewise.  (Moved from tree-data-ref.h.)
> (initialize_data_dependence_relation): Use XCNEW and remove
> explicit zeroing of DDR_REVERSED_P.  Look for a subsequence
> of access functions that have the same type.  Allow the
> subsequence to end with different bases in some circumstances.
> Record the chosen access functions in SUB_ACCESS_FN.
> (build_classic_dist_vector_1): Replace ddr_a and ddr_b with
> a_index and b_index.  Use SUB_ACCESS_FN instead of DR_ACCESS_FN.
> (subscript_dependence_tester_1): Likewise dra and drb.
> (build_classic_dist_vector): Update calls accordingly.
> (subscript_dependence_tester): Likewise.
> * tree-ssa-loop-prefetch.c (determine_loop_nest_reuse): Check
> DDR_COULD_BE_INDEPENDENT_P.
> * tree-vectorizer.h (LOOP_REQUIRES_VERSIONING_FOR_ALIAS): Test
> comp_alias_ddrs instead of may_alias_ddrs.
> * tree-vect-data-refs.c (vect_analyze_possibly_independent_ddr):
> New function.
> (vect_analyze_data_ref_dependence): Use it if
> DDR_COULD_BE_INDEPENDENT_P, but fall back to using the recorded
> distance vectors if that fails.
> (dependence_distance_ge_vf): New function.
> (vect_prune_runtime_alias_test_list): Use it.  Don't clear
> LOOP_VINFO_MAY_ALIAS_DDRS.
>
> gcc/testsuite/
> * gcc.dg/vect/vect-alias-check-3.c: New test.
> * gcc.dg/vect/vect-alias-check-4.c: Likewise.
> * gcc.dg/vect/vect-alias-check-5.c: Likewise.
>
> Index: gcc/tree-data-ref.h
> ===
> --- gcc/tree-data-ref.h 2017-07-27 13:10:29.620045506 +0100
> +++ gcc/tree-data-ref.h 2017-07-27 13:10:33.023912613 +0100
> @@ -260,6 +260,9 @@ struct conflict_function
>
>  struct subscript
>  {
> +  /* The access functions of the two references.  */
> +  tree access_fn[2];
> +
>/* A description of the iterations for which the elements are
>   accessed twice.  */
>conflict_function *conflicting_iterations_in_a;
> @@ -278,6 +281,7 @@ struct subscript
>
>  typedef struct subscript *subscript_p;
>
> +

Re: [PATCH] Fold (int *)&a + 4 to a[1] using offset_int (PR middle-end/81695)

[Richard Biener]

On Fri, Aug 4, 2017 at 10:38 AM, Marek Polacek  wrote:
> We were crashing because size_binop_loc got operands of different types:
> sizetype and ssizetype.  Richi suggested performing the computation in
> offset_int which my patch tries to do.  Unsure about the sdiv_trunc part,
> what do I use instead of EXACT_DIV_EXPR?

Use wi::divmod_trunc and check for a zero remainder.  If the remainder is not
zero we may not fold.

Ok with that change.

Thanks,
Richard.

> Bootstrapped/regtested on x86_64-linux, ok for trunk?  Testing on ppc64le
> in progress.
>
> 2017-08-04  Marek Polacek  
>
> PR middle-end/81695
> * fold-const.c (fold_indirect_ref_1): For ((int *)&a + 4 -> a[1],
> perform the computation in offset_int.
>
> * gcc.dg/pr81695.c: New test.
>
> diff --git gcc/fold-const.c gcc/fold-const.c
> index ed6c289a64b..64a84eb0197 100644
> --- gcc/fold-const.c
> +++ gcc/fold-const.c
> @@ -14106,14 +14106,17 @@ fold_indirect_ref_1 (location_t loc, tree type, 
> tree op0)
>&& type == TREE_TYPE (op00type))
> {
>   tree type_domain = TYPE_DOMAIN (op00type);
> - tree min_val = size_zero_node;
> - if (type_domain && TYPE_MIN_VALUE (type_domain))
> -   min_val = TYPE_MIN_VALUE (type_domain);
> - op01 = size_binop_loc (loc, EXACT_DIV_EXPR, op01,
> -TYPE_SIZE_UNIT (type));
> - op01 = size_binop_loc (loc, PLUS_EXPR, op01, min_val);
> - return build4_loc (loc, ARRAY_REF, type, op00, op01,
> -NULL_TREE, NULL_TREE);
> + tree min = TYPE_MIN_VALUE (type_domain);
> + if (min && TREE_CODE (min) == INTEGER_CST)
> +   {
> + offset_int off = wi::to_offset (op01);
> + offset_int el_sz = wi::to_offset (TYPE_SIZE_UNIT (type));
> + off = wi::sdiv_trunc (off, el_sz);
> + off = off + wi::to_offset (min);
> + op01 = wide_int_to_tree (sizetype, off);
> + return build4_loc (loc, ARRAY_REF, type, op00, op01,
> +NULL_TREE, NULL_TREE);
> +   }
> }
> }
>  }
> diff --git gcc/testsuite/gcc.dg/pr81695.c gcc/testsuite/gcc.dg/pr81695.c
> index e69de29bb2d..c3452580f1c 100644
> --- gcc/testsuite/gcc.dg/pr81695.c
> +++ gcc/testsuite/gcc.dg/pr81695.c
> @@ -0,0 +1,11 @@
> +/* PR middle-end/81695 */
> +/* { dg-do compile } */
> +/* { dg-options "" } */
> +
> +int z[] = { };
> +
> +int
> +main (void)
> +{
> +  __builtin_printf ("%d\n", *(z + 1));
> +}
>
> Marek

Re: [PATCHv5][PR 57371] Remove useless floating point casts in comparisons

[Richard Biener]

On Sun, Jul 30, 2017 at 9:25 AM, Yury Gribov
 wrote:
> Hi all,
>
> This is an updated version of patch in
> https://gcc.gnu.org/ml/gcc-patches/2017-07/msg00924.html . It prevents
> optimization in presense of sNaNs (and qNaNs when comparison operator is >
>>= < <=) to preserve FP exceptions.
>
> I've fixed some stylistic issues mentioned in previous review.
>
> Rebased, bootstrapped and regtested on x64. Ok for trunk?

Ok.

Thanks,
Richard.

> -Y

Re: [PATCH] Verify edge probability consistency in verify_flow_info

[Jan Hubicka]

> > 
> > III.
> > 
> > I've written this patch to check for the missing probability more
> > consistently. I'm not certain if we can require that the probability
> > should always be set, so I'm just requiring that if it is set on one
> > outgoing edge, it needs to be set on all outgoing edges.
> > 
> > Sofar I've build a c-only x86_64 non-bootstrap compiler and ran dg.exp.
> > The only problem I ran into was in attr-simd{,-2,-4}.c. I've written a
> > tentative patch for that, and will submit it as follow-up.
> > 
> > Is this check a good idea?
> I think the additional checking is a good idea.  Ideally we'd verify
> that all edges have a probability.  Until then I think you need some
> kind of rationale in a comment for why the checking is limited.
> 
> > 
> > OK for trunk if bootstrap and reg-test on x86_64 succeeds?
> Yea, but I'd like to see ongoing work towards full checking.

I have full checking in my tree for some time.  At x86-64 bootstrap there
is one remaining offender in simd_clone_adjust which was not fixed yet
https://gcc.gnu.org/ml/gcc-patches/2017-07/msg00219.html
Jakub did not tell me what would be a reasonable guess :)

After that I plan to enable full checking after checking arm/ppc.
So I hope we will converge to full checking really soon.  But having
additional check is fine.

Honza
> 
> Jeff

Re: Handle data dependence relations with different bases

[Richard Sandiford]

Richard Biener  writes:
> On Thu, Jul 27, 2017 at 2:19 PM, Richard Sandiford
>  wrote:
>> Richard Sandiford  writes:
>>> Eric Botcazou  writes:
 [Sorry for missing the previous messages]

> Thanks.  Just been retesting, and I think I must have forgotten
> to include Ada last time.  It turns out that the patch causes a dg-scan
> regression in gnat.dg/vect17.adb, because we now think that if the
> array RECORD_TYPEs *do* alias in:
>
>procedure Add (X, Y : aliased Sarray; R : aliased out Sarray) is
>begin
>   for I in Sarray'Range loop
>  R(I) := X(I) + Y(I);
>   end loop;
>end;
>
> then the dependence distance must be zero.  Eric, does that hold true
> for Ada?  I.e. if X and R (or Y and R) alias, must it be the case that
> X(I) can only alias R(I) and not for example R(I-1) or R(I+1)?

 Yes, I'd think so (even without the artificial RECORD_TYPE around
> the arrays).
>>>
>>> Good!
>>>
> 2017-06-07  Richard Sandiford  
>
> gcc/testsuite/
> * gnat.dg/vect17.ads (Sarray): Increase range to 1 .. 5.
> * gnat.dg/vect17.adb (Add): Create a dependence distance of 1
> when X = R or Y = R.

 I think that you need to modify vect15 and vect16 the same way.
>>>
>>> Ah, yeah.  And doing that shows that I'd not handled safelen for
>>> DDR_COULD_BE_INDEPENDENT_P.  I've fixed that locally.
>>>
>>> How does this look?  Tested on x86_64-linux-gnu both without the
>>> vectoriser changes and with the fixed vectoriser patch.
>>
>> Here's a version of the patch that handles safelen.  I split the
>> handling out into a new function (vect_analyze_possibly_independent_ddr)
>> since it was getting too big to do inline.
>>
>> Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?
>
> Ok.

Thanks!

> Did you check whether BB vectorization is affected?  See
> vect_slp_analyze_instance_dependence
> and friends.  It's quite conservative but given the prefetching change
> I wonder if we need
> to rule out DDR_COULD_BE_INDEPENDENT_P?

I think it should be OK.  When DDR_COULD_BE_INDEPENDENT_P is set,
we've effectively changed from DDR_ARE_DEPENDENT == chrec_dont_know
to a conservatively-correct distance vector.  It looks like
vect_slp_analyze_data_ref_dependence handles both cases in the
same way (by returning true).

Thanks,
Richard

>
> Thanks,
> Richard.
>
>> Thanks,
>> Richard
>>
>>
>> 2017-07-27  Richard Sandiford  
>>
>> gcc/
>> * tree-data-ref.h (subscript): Add access_fn field.
>> (data_dependence_relation): Add could_be_independent_p.
>> (SUB_ACCESS_FN, DDR_COULD_BE_INDEPENDENT_P): New macros.
>> (same_access_functions): Move to tree-data-ref.c.
>> * tree-data-ref.c (ref_contains_union_access_p): New function.
>> (access_fn_component_p): Likewise.
>> (access_fn_components_comparable_p): Likewise.
>> (dr_analyze_indices): Add a reference to access_fn_component_p.
>> (dump_data_dependence_relation): Use SUB_ACCESS_FN instead of
>> DR_ACCESS_FN.
>> (constant_access_functions): Likewise.
>> (add_other_self_distances): Likewise.
>> (same_access_functions): Likewise.  (Moved from tree-data-ref.h.)
>> (initialize_data_dependence_relation): Use XCNEW and remove
>> explicit zeroing of DDR_REVERSED_P.  Look for a subsequence
>> of access functions that have the same type.  Allow the
>> subsequence to end with different bases in some circumstances.
>> Record the chosen access functions in SUB_ACCESS_FN.
>> (build_classic_dist_vector_1): Replace ddr_a and ddr_b with
>> a_index and b_index.  Use SUB_ACCESS_FN instead of DR_ACCESS_FN.
>> (subscript_dependence_tester_1): Likewise dra and drb.
>> (build_classic_dist_vector): Update calls accordingly.
>> (subscript_dependence_tester): Likewise.
>> * tree-ssa-loop-prefetch.c (determine_loop_nest_reuse): Check
>> DDR_COULD_BE_INDEPENDENT_P.
>> * tree-vectorizer.h (LOOP_REQUIRES_VERSIONING_FOR_ALIAS): Test
>> comp_alias_ddrs instead of may_alias_ddrs.
>> * tree-vect-data-refs.c (vect_analyze_possibly_independent_ddr):
>> New function.
>> (vect_analyze_data_ref_dependence): Use it if
>> DDR_COULD_BE_INDEPENDENT_P, but fall back to using the recorded
>> distance vectors if that fails.
>> (dependence_distance_ge_vf): New function.
>> (vect_prune_runtime_alias_test_list): Use it.  Don't clear
>> LOOP_VINFO_MAY_ALIAS_DDRS.
>>
>> gcc/testsuite/
>> * gcc.dg/vect/vect-alias-check-3.c: New test.
>> * gcc.dg/vect/vect-alias-check-4.c: Likewise.
>> * gcc.dg/vect/vect-alias-check-5.c: Likewise.
>>
>> Index: gcc/tree-data-ref.h
>> ===
>> --- gcc/tree-data-ref.h 2017-07-27 13:10:29.620045506 +0100
>>

Re: Handle data dependence relations with different bases

[Richard Biener]

On Fri, Aug 4, 2017 at 11:28 AM, Richard Sandiford
 wrote:
> Richard Biener  writes:
>> On Thu, Jul 27, 2017 at 2:19 PM, Richard Sandiford
>>  wrote:
>>> Richard Sandiford  writes:
 Eric Botcazou  writes:
> [Sorry for missing the previous messages]
>
>> Thanks.  Just been retesting, and I think I must have forgotten
>> to include Ada last time.  It turns out that the patch causes a dg-scan
>> regression in gnat.dg/vect17.adb, because we now think that if the
>> array RECORD_TYPEs *do* alias in:
>>
>>procedure Add (X, Y : aliased Sarray; R : aliased out Sarray) is
>>begin
>>   for I in Sarray'Range loop
>>  R(I) := X(I) + Y(I);
>>   end loop;
>>end;
>>
>> then the dependence distance must be zero.  Eric, does that hold true
>> for Ada?  I.e. if X and R (or Y and R) alias, must it be the case that
>> X(I) can only alias R(I) and not for example R(I-1) or R(I+1)?
>
> Yes, I'd think so (even without the artificial RECORD_TYPE around
>> the arrays).

 Good!

>> 2017-06-07  Richard Sandiford  
>>
>> gcc/testsuite/
>> * gnat.dg/vect17.ads (Sarray): Increase range to 1 .. 5.
>> * gnat.dg/vect17.adb (Add): Create a dependence distance of 1
>> when X = R or Y = R.
>
> I think that you need to modify vect15 and vect16 the same way.

 Ah, yeah.  And doing that shows that I'd not handled safelen for
 DDR_COULD_BE_INDEPENDENT_P.  I've fixed that locally.

 How does this look?  Tested on x86_64-linux-gnu both without the
 vectoriser changes and with the fixed vectoriser patch.
>>>
>>> Here's a version of the patch that handles safelen.  I split the
>>> handling out into a new function (vect_analyze_possibly_independent_ddr)
>>> since it was getting too big to do inline.
>>>
>>> Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?
>>
>> Ok.
>
> Thanks!
>
>> Did you check whether BB vectorization is affected?  See
>> vect_slp_analyze_instance_dependence
>> and friends.  It's quite conservative but given the prefetching change
>> I wonder if we need
>> to rule out DDR_COULD_BE_INDEPENDENT_P?
>
> I think it should be OK.  When DDR_COULD_BE_INDEPENDENT_P is set,
> we've effectively changed from DDR_ARE_DEPENDENT == chrec_dont_know
> to a conservatively-correct distance vector.  It looks like
> vect_slp_analyze_data_ref_dependence handles both cases in the
> same way (by returning true).

Yes.  Could be improved of course.

Thanks for double-checking.
Richard.

> Thanks,
> Richard
>
>>
>> Thanks,
>> Richard.
>>
>>> Thanks,
>>> Richard
>>>
>>>
>>> 2017-07-27  Richard Sandiford  
>>>
>>> gcc/
>>> * tree-data-ref.h (subscript): Add access_fn field.
>>> (data_dependence_relation): Add could_be_independent_p.
>>> (SUB_ACCESS_FN, DDR_COULD_BE_INDEPENDENT_P): New macros.
>>> (same_access_functions): Move to tree-data-ref.c.
>>> * tree-data-ref.c (ref_contains_union_access_p): New function.
>>> (access_fn_component_p): Likewise.
>>> (access_fn_components_comparable_p): Likewise.
>>> (dr_analyze_indices): Add a reference to access_fn_component_p.
>>> (dump_data_dependence_relation): Use SUB_ACCESS_FN instead of
>>> DR_ACCESS_FN.
>>> (constant_access_functions): Likewise.
>>> (add_other_self_distances): Likewise.
>>> (same_access_functions): Likewise.  (Moved from tree-data-ref.h.)
>>> (initialize_data_dependence_relation): Use XCNEW and remove
>>> explicit zeroing of DDR_REVERSED_P.  Look for a subsequence
>>> of access functions that have the same type.  Allow the
>>> subsequence to end with different bases in some circumstances.
>>> Record the chosen access functions in SUB_ACCESS_FN.
>>> (build_classic_dist_vector_1): Replace ddr_a and ddr_b with
>>> a_index and b_index.  Use SUB_ACCESS_FN instead of DR_ACCESS_FN.
>>> (subscript_dependence_tester_1): Likewise dra and drb.
>>> (build_classic_dist_vector): Update calls accordingly.
>>> (subscript_dependence_tester): Likewise.
>>> * tree-ssa-loop-prefetch.c (determine_loop_nest_reuse): Check
>>> DDR_COULD_BE_INDEPENDENT_P.
>>> * tree-vectorizer.h (LOOP_REQUIRES_VERSIONING_FOR_ALIAS): Test
>>> comp_alias_ddrs instead of may_alias_ddrs.
>>> * tree-vect-data-refs.c (vect_analyze_possibly_independent_ddr):
>>> New function.
>>> (vect_analyze_data_ref_dependence): Use it if
>>> DDR_COULD_BE_INDEPENDENT_P, but fall back to using the recorded
>>> distance vectors if that fails.
>>> (dependence_distance_ge_vf): New function.
>>> (vect_prune_runtime_alias_test_list): Use it.  Don't clear
>>> LOOP_VINFO_MAY_ALIAS_DDRS.
>>>
>>> gcc/testsuite/
>>> * gcc.dg/vect/vect-alias-check-3.c: New test.
>>> * gcc.dg/vect

[C++ Patch Ping] Re: [C++ Patch] PR 79790 ("[C++17] ICE class template argument deduction")

[Paolo Carlini]

Hi,

On 14/07/2017 19:51, Nathan Sidwell wrote:

On 07/14/2017 01:32 PM, Paolo Carlini wrote:

While working on the bug I also noticed that we can simplify a bit 
the code
generating the implicit deduction guides: if I'm not mistaken, when 
we pass
types as first argument of build_deduction_guide - for implicit 
guides, that is
- the deduction guide is never explicit. thus DECL_NONCONVERTING_P is 
never
true. It's an unrelated tweak, anyway, which we can consider applying 
by itself

if we don't change the code generating the implicit deduction guides.


I recall wondering the same thing when adding the 'elided = true' 
pieces, but didn't investigate enough to confirm/deny.  Thanks for 
getting this.

You are welcome!

I think the rest of the patch - the bug fix proper - still awaits a 
review...


Thanks!
Paolo.

Re: [PATCH] Fold (int *)&a + 4 to a[1] using offset_int (PR middle-end/81695)

[Marek Polacek]

On Fri, Aug 04, 2017 at 11:08:49AM +0200, Richard Biener wrote:
> On Fri, Aug 4, 2017 at 10:38 AM, Marek Polacek  wrote:
> > We were crashing because size_binop_loc got operands of different types:
> > sizetype and ssizetype.  Richi suggested performing the computation in
> > offset_int which my patch tries to do.  Unsure about the sdiv_trunc part,
> > what do I use instead of EXACT_DIV_EXPR?
> 
> Use wi::divmod_trunc and check for a zero remainder.  If the remainder is not
> zero we may not fold.
> 
> Ok with that change.

Thanks, will commit this after the usual testing:

2017-08-04  Marek Polacek  

PR middle-end/81695
* fold-const.c (fold_indirect_ref_1): For ((int *)&a + 4 -> a[1],
perform the computation in offset_int.

* gcc.dg/pr81695.c: New test.

diff --git gcc/fold-const.c gcc/fold-const.c
index ed6c289a64b..1f55bee8fc0 100644
--- gcc/fold-const.c
+++ gcc/fold-const.c
@@ -14106,14 +14106,21 @@ fold_indirect_ref_1 (location_t loc, tree type, tree 
op0)
   && type == TREE_TYPE (op00type))
{
  tree type_domain = TYPE_DOMAIN (op00type);
- tree min_val = size_zero_node;
- if (type_domain && TYPE_MIN_VALUE (type_domain))
-   min_val = TYPE_MIN_VALUE (type_domain);
- op01 = size_binop_loc (loc, EXACT_DIV_EXPR, op01,
-TYPE_SIZE_UNIT (type));
- op01 = size_binop_loc (loc, PLUS_EXPR, op01, min_val);
- return build4_loc (loc, ARRAY_REF, type, op00, op01,
-NULL_TREE, NULL_TREE);
+ tree min = TYPE_MIN_VALUE (type_domain);
+ if (min && TREE_CODE (min) == INTEGER_CST)
+   {
+ offset_int off = wi::to_offset (op01);
+ offset_int el_sz = wi::to_offset (TYPE_SIZE_UNIT (type));
+ offset_int remainder;
+ off = wi::divmod_trunc (off, el_sz, SIGNED, &remainder);
+ if (remainder == 0)
+   {
+ off = off + wi::to_offset (min);
+ op01 = wide_int_to_tree (sizetype, off);
+ return build4_loc (loc, ARRAY_REF, type, op00, op01,
+NULL_TREE, NULL_TREE);
+   }
+   }
}
}
 }
diff --git gcc/testsuite/gcc.dg/pr81695.c gcc/testsuite/gcc.dg/pr81695.c
index e69de29bb2d..c3452580f1c 100644
--- gcc/testsuite/gcc.dg/pr81695.c
+++ gcc/testsuite/gcc.dg/pr81695.c
@@ -0,0 +1,11 @@
+/* PR middle-end/81695 */
+/* { dg-do compile } */
+/* { dg-options "" } */
+
+int z[] = { };
+
+int
+main (void)
+{
+  __builtin_printf ("%d\n", *(z + 1));
+}

Marek

[WIP] Possible Bug in vect_bb_slp_scalar_cost?

[Dominik Inführ]

Hi,

vect_bb_slp_scalar_cost computes the scalar cost of a SLP node. If there are 
non-scalar uses of a definition, the costs for it and its operands (children) 
are ignored. The vector LIFE is used to keep track of this and an element is 
set to true, such that the def and its children are ignored. But as soon as an 
element is set to true it is never undone, that means the following sibling and 
parent nodes of the current node also stay ignored. This seems wrong to me, a 
simple fix would be to clone LIFE for every vector, such that changes to LIFE 
stay in the subtree:

diff --git a/gcc/tree-vect-slp.c b/gcc/tree-vect-slp.c
index 032a9444a5a..f919645f28b 100644
--- a/gcc/tree-vect-slp.c
+++ b/gcc/tree-vect-slp.c
@@ -2590,6 +2590,11 @@ vect_bb_slp_scalar_cost (basic_block bb,
   gimple *stmt;
   slp_tree child;

+  auto_vec subtree_life;
+  subtree_life.safe_splice (*life);
+
+  life = &subtree_life;
+
   FOR_EACH_VEC_ELT (SLP_TREE_SCALAR_STMTS (node), i, stmt)
 {
   unsigned stmt_cost;




signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: [PATCH] Verify edge probability consistency in verify_flow_info

[Tom de Vries]

On 08/04/2017 11:15 AM, Jan Hubicka wrote:

OK for trunk if bootstrap and reg-test on x86_64 succeeds?

Yea, but I'd like to see ongoing work towards full checking.


I have full checking in my tree for some time.  At x86-64 bootstrap there
is one remaining offender in simd_clone_adjust which was not fixed yet
https://gcc.gnu.org/ml/gcc-patches/2017-07/msg00219.html
Jakub did not tell me what would be a reasonable guess :)


I think I just fixed that one here ( 
https://gcc.gnu.org/ml/gcc-cvs/2017-08/msg00112.html ).


Thanks,
- Tom

Re: [PATCH][GCC] Simplification of 1U << (31 - x)

[Richard Biener]

On Tue, Aug 1, 2017 at 11:14 AM, Sudi Das  wrote:
>
>
>
>
> Sorry about the delayed response but looking at the above discussion, should 
> I conclude that this is a valid tree simplification?

Yes, I think so.  Jakub requested code to undo this at RTL expansion
based on target costs, not sure if we really should
require that from you given the user could have written the target
sequence himself.

Few comments about the patch:

+/* Fold (1 << (C - x)) where C = precision(type) - 1
+   into ((1 << C) >> x). */
+(simplify
+ (lshift integer_onep@0 (minus INTEGER_CST@1 @2))

I think this warrants a single_use check on the minus (note :s isn't enough
as with the unsigned case we'd happily ignore it by design).

+  (if (INTEGRAL_TYPE_P (type)
+   && TYPE_PRECISION (type) <= HOST_BITS_PER_WIDE_INT
+   && tree_to_uhwi (@1) == (unsigned)(TYPE_PRECISION (type) - 1))

You can relax this with using

  && wi::eq_p (@1, TYPE_PRECISION (type) - 1)

+   (if (TYPE_UNSIGNED(type))
+ (rshift (lshift @0 @1) @2)
+   (with
+{ tree utype = unsigned_type_for (type); }
+(convert:type (rshift (lshift (convert:utype @0) @1) @2))
+

You can write (convert (rshift ...)), without the :type.

I'm leaving it to Jakub whether you need to write that RTL expansion tweak.

Thanks,
Richard.

> I am pasting the diff of the assembly that AArch64 generates with the test 
> case that I added. I see fewer instructions generated with the patch.
>
> --- pr80131-1.s2017-08-01 10:02:43.243374174 +0100
> +++ pr80131-1.s-patched2017-08-01 10:00:54.776455630 +0100
> @@ -24,10 +24,8 @@
>  strx0, [sp, 8]
>  ldrx0, [sp, 8]
>  movw1, w0
> -movw0, 63
> -subw0, w0, w1
> -movx1, 1
> -lslx0, x1, x0
> +movx0, -9223372036854775808
> +lsrx0, x0, x1
>  addsp, sp, 16
>  ret
>  .sizef2, .-f2
> @@ -39,10 +37,8 @@
>  strx0, [sp, 8]
>  ldrx0, [sp, 8]
>  movw1, w0
> -movw0, 63
> -subw0, w0, w1
> -movx1, 1
> -lslx0, x1, x0
> +movx0, -9223372036854775808
> +lsrx0, x0, x1
>  addsp, sp, 16
>  ret
>  .sizef3, .-f3
> @@ -52,11 +48,9 @@
>  f4:
>  subsp, sp, #16
>  strw0, [sp, 12]
> -movw1, 31
>  ldrw0, [sp, 12]
> -subw0, w1, w0
> -movw1, 1
> -lslw0, w1, w0
> +movw1, -2147483648
> +lsrw0, w1, w0
>  addsp, sp, 16
>  ret
>  .sizef4, .-f4
>
>
> Thanks
>
> Sudi
>
>
>
>
> From: Wilco Dijkstra
> Sent: Thursday, April 13, 2017 1:01 PM
> To: Richard Biener; Jakub Jelinek
> Cc: Sudi Das; GCC Patches; nd; Richard Earnshaw; James Greenhalgh
> Subject: Re: [PATCH][GCC] Simplification of 1U << (31 - x)
>
> Richard Biener wrote:
>> It is IMHO a valid GIMPLE optimization / canonicalization.
>>
>>movabsq $-9223372036854775808, %rax
>>
>> so this should then have been generated as 1<<63?
>>
>> At some point variable shifts were quite expensive as well..
>
> Yes I don't see a major difference between movabsq and
>
> movl$1, %eax
> salq$63, %rax
>
> on my Sandy Bridge, but if the above is faster then that is what the x64
> backend should emit - it's 1 byte smaller as well, so probably better in all
> cases.
>
> Wilco

Re: [WIP] Possible Bug in vect_bb_slp_scalar_cost?

[Richard Biener]

On Fri, Aug 4, 2017 at 12:08 PM, Dominik Inführ
 wrote:
> Hi,
>
> vect_bb_slp_scalar_cost computes the scalar cost of a SLP node. If there are 
> non-scalar uses of a definition, the costs for it and its operands (children) 
> are ignored. The vector LIFE is used to keep track of this and an element is 
> set to true, such that the def and its children are ignored. But as soon as 
> an element is set to true it is never undone, that means the following 
> sibling and parent nodes of the current node also stay ignored.

Yes, that's intended.  They are live as well because they are needed
to compute the live scalar.

This seems wrong to me, a simple fix would be to clone LIFE for every
vector, such that changes to LIFE stay in the subtree:
>
> diff --git a/gcc/tree-vect-slp.c b/gcc/tree-vect-slp.c
> index 032a9444a5a..f919645f28b 100644
> --- a/gcc/tree-vect-slp.c
> +++ b/gcc/tree-vect-slp.c
> @@ -2590,6 +2590,11 @@ vect_bb_slp_scalar_cost (basic_block bb,
>gimple *stmt;
>slp_tree child;
>
> +  auto_vec subtree_life;
> +  subtree_life.safe_splice (*life);
> +
> +  life = &subtree_life;
> +
>FOR_EACH_VEC_ELT (SLP_TREE_SCALAR_STMTS (node), i, stmt)
>  {
>unsigned stmt_cost;
>
>

RE: [PATCH] i386: Rewrite check for AVX512 features

[Peryt, Sebastian]

> -Original Message-
> From: Uros Bizjak [mailto:ubiz...@gmail.com]
> Sent: Sunday, July 30, 2017 11:02 AM
> To: H.J. Lu 
> Cc: gcc-patches@gcc.gnu.org; Koval, Julia ; Peryt,
> Sebastian 
> Subject: Re: [PATCH] i386: Rewrite check for AVX512 features
> 
> On Sat, Jul 29, 2017 at 3:06 PM, H.J. Lu  wrote:
> > Add a new file, avx512-check.h, to check all AVX512 features.  The
> > test is skipped if any requested AVX512 features are unavailable.
> >
> > Tested on Skylake server and Haswell.  OK for trunk?
> 
> No, I'd rather leave it in in the way they are now, so test can include 
> individual
> checks.
> 
> Uros.
> 
Uros,

Can you please suggests any alternative approach? The main problem with current
one used in avx512f-helper.h is that it doesn't take into account situations 
where
two features are required, but only one is supported by CPU.

That's exactly the case with AVX512VL and AVX512VBMI on SKX. Once 
avx512vl-check.h
verifies existence of AVX512VL on SKX it starts to execute test, which fails 
because 
AVX512VBMI is not supported but it has never been checked, before test 
execution.

Honestly I cannot think of any solution that would allow for both individual 
include 
files (beside what HJ already did in those few remaining tests) and multiple 
features
verification. Also I think it's worth taking into account that not many tests 
actually 
use individual include files instead of avx512f-helper.h.

Thanks,
Sebastian
> >
> > H.J.
> > ---
> > PR target/81590
> > * gcc.target/i386/avx512-check.h: New file.
> > * gcc.target/i386/avx5124fmaps-check.h: Removed.
> > * gcc.target/i386/avx5124vnniw-check.h: Likewise.
> > * gcc.target/i386/avx512cd-check.h: Likewise.
> > * gcc.target/i386/avx512ifma-check.h: Likewise.
> > * gcc.target/i386/avx512vbmi-check.h: Likewise.
> > * gcc.target/i386/avx512vpopcntdq-check.h: Likewise.
> > * gcc.target/i386/avx512bw-check.h: Rewrite.
> > * gcc.target/i386/avx512dq-check.h: Likewise.
> > * gcc.target/i386/avx512er-check.h: Likewise.
> > * gcc.target/i386/avx512f-check.h: Likewise.
> > * gcc.target/i386/avx512vl-check.h: Likewise.
> > * gcc.target/i386/avx512f-helper.h: Include "avx512-check.h"
> > only.
> > (test_512): Removed.
> > (avx512*_test): Likewise.
> > * gcc.target/i386/avx512f-pr71559.c (TEST): Undef.
> > ---
> >  gcc/testsuite/gcc.target/i386/avx512-check.h   | 113
> +
> >  gcc/testsuite/gcc.target/i386/avx5124fmaps-check.h |  47 -
> > gcc/testsuite/gcc.target/i386/avx5124vnniw-check.h |  47 -
> >  gcc/testsuite/gcc.target/i386/avx512bw-check.h |  50 +
> >  gcc/testsuite/gcc.target/i386/avx512cd-check.h |  46 -
> >  gcc/testsuite/gcc.target/i386/avx512dq-check.h |  50 +
> >  gcc/testsuite/gcc.target/i386/avx512er-check.h |  49 +
> >  gcc/testsuite/gcc.target/i386/avx512f-check.h  |  49 +
> >  gcc/testsuite/gcc.target/i386/avx512f-helper.h |  64 +---
> >  gcc/testsuite/gcc.target/i386/avx512f-pr71559.c|   1 +
> >  gcc/testsuite/gcc.target/i386/avx512ifma-check.h   |  46 -
> >  gcc/testsuite/gcc.target/i386/avx512vbmi-check.h   |  46 -
> >  gcc/testsuite/gcc.target/i386/avx512vl-check.h |  51 +-
> >  .../gcc.target/i386/avx512vpopcntdq-check.h|  47 -
> >  14 files changed, 130 insertions(+), 576 deletions(-)  create mode
> > 100644 gcc/testsuite/gcc.target/i386/avx512-check.h
> >  delete mode 100644 gcc/testsuite/gcc.target/i386/avx5124fmaps-check.h
> >  delete mode 100644 gcc/testsuite/gcc.target/i386/avx5124vnniw-check.h
> >  delete mode 100644 gcc/testsuite/gcc.target/i386/avx512cd-check.h
> >  delete mode 100644 gcc/testsuite/gcc.target/i386/avx512ifma-check.h
> >  delete mode 100644 gcc/testsuite/gcc.target/i386/avx512vbmi-check.h
> >  delete mode 100644
> > gcc/testsuite/gcc.target/i386/avx512vpopcntdq-check.h
> >
> > diff --git a/gcc/testsuite/gcc.target/i386/avx512-check.h
> > b/gcc/testsuite/gcc.target/i386/avx512-check.h
> > new file mode 100644
> > index 000..bfe14960100
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/i386/avx512-check.h
> > @@ -0,0 +1,113 @@
> > +#include 
> > +#include "cpuid.h"
> > +#include "m512-check.h"
> > +#include "avx512f-os-support.h"
> > +
> > +#ifndef DO_TEST
> > +#define DO_TEST do_test
> > +#ifdef AVX512VL
> > +static void test_256 (void);
> > +static void test_128 (void);
> > +#else
> > +static void test_512 (void);
> > +#endif
> > +
> > +__attribute__ ((noinline))
> > +static void
> > +do_test (void)
> > +{
> > +#ifdef AVX512VL
> > +  test_256 ();
> > +  test_128 ();
> > +#else
> > +  test_512 ();
> > +#endif
> > +}
> > +#endif
> > +
> > +int
> > +main ()
> > +{
> > +  unsigned int eax, ebx, ecx, edx;
> > +
> > +  if (!__get_cpuid (1, &eax, &ebx, &ecx, &edx))
> > +goto skipped;
> > +
> > +  /

[PATCH] Fix PR81705

[Richard Biener]

The following fixes PR81705, a simple omission in my last association 
patch.

Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.

Richard.

2017-08-04  Richard Biener  

PR middle-end/81705
* fold-const.c (fold_binary_loc): Properly restrict
minus_var0 && minus_var1 case when associating undefined overflow
entities.

* c-c++-common/ubsan/pr81705.c: New testcase.

Index: gcc/fold-const.c
===
--- gcc/fold-const.c(revision 250865)
+++ gcc/fold-const.c(working copy)
@@ -9592,12 +9592,13 @@ fold_binary_loc (location_t loc,
  if (POINTER_TYPE_P (atype)
  || (INTEGRAL_TYPE_P (atype) && !TYPE_OVERFLOW_WRAPS (atype)))
{
- if (var0 && var1)
+ if ((var0 && var1) || (minus_var0 && minus_var1))
{
  /* ???  If split_tree would handle NEGATE_EXPR we could
-simplify this down to the var0/minus_var1 cases.  */
- tree tmp0 = var0;
- tree tmp1 = var1;
+simply reject these cases and the allowed cases would
+be the var0/minus_var1 ones.  */
+ tree tmp0 = var0 ? var0 : minus_var0;
+ tree tmp1 = var1 ? var1 : minus_var1;
  bool one_neg = false;
 
  if (TREE_CODE (tmp0) == NEGATE_EXPR)
Index: gcc/testsuite/c-c++-common/ubsan/pr81705.c
===
--- gcc/testsuite/c-c++-common/ubsan/pr81705.c  (nonexistent)
+++ gcc/testsuite/c-c++-common/ubsan/pr81705.c  (working copy)
@@ -0,0 +1,12 @@
+/* { dg-do run } */
+/* { dg-options "-fsanitize=undefined -fsanitize-undefined-trap-on-error" } */
+
+int var_4 = -1716607962;
+int var_14 = 943738830;
+volatile int a;
+int main()
+{
+  //  (-(-1716607962) - 516151698) - -(9403738830)
+  a = (-var_4 - 516151698) - -var_14;
+  return 0;
+}

Re: gotools patch committed: Test runtime, misc/cgo/{test,testcarchive}

[Uros Bizjak]

On Wed, Jul 26, 2017 at 10:26 PM, Ian Lance Taylor  wrote:
> On Sat, Jul 22, 2017 at 11:08 AM, Uros Bizjak  wrote:
>>> This patch to the gotools Makefile adds tests to `make check`.  We now
>>> test the runtime package using the newly built go tool, and test that
>>> cgo works by running the misc/cgo/test and misc/cgo/testcarchive
>>> tests.  Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.
>>> Committed to mainline.
>>
>> There is now only one remaining gotools testsuite failure on alpha:
>>
>> FAIL: TestBreakpoint
>> crash_test.go:106: testprog Breakpoint exit status: exit status 2
>> crash_test.go:310: output:
>> SIGTRAP: trace trap
>> PC=2199039253124 m=0 sigcode=0
>>
>> goroutine 1 [running]:
>>
>> goroutine 3 [runnable]:
>> created by runtime.SetFinalizer
>>
>> /space/homedirs/uros/gcc-svn/trunk/libgo/go/runtime/mfinal.go:355
>> +1280
>>
>>
>>
>> want output containing: runtime.Breakpoint
>>
>> I would like to debug this one failure only. Is there a way to run
>> only one gotools test? Can you perhaps give a hint where to look in
>> the source?
>
> The test is TestBreakpoint in libgo/go/runtime/crash_test.go.  It is
> testing that if it runs a program that calls `runtime.Breakpoint`,
> then `runtime.Breakpoint` will appear in the stack trace that the
> program emits.
>
> It does this by building a test program.  The easy way to do this
> yourself is to run `make install` in your GCC build directory, set
> LD_LIBRARY_PATH if needed to include the newly installed libgo.so, and
> then do
>
> cd SRCDIR/libgo/go/runtime/testdata/testprog
> go build# run the `go` program installed from gotools, building
> ./testprog; you can use `go build -o /tmp/x` if you like
> ./testprog Breakpoint
>
> On my x86_64 system that prints the appended, which includes the
> desired `runtime.Breakpoint` string.  On your system it fails to print
> a stack trace, but I don't know why.

The problem was following:

runtime.Breakpoint comprises only call to __builtin_trap (), which in
case of alpha maps to "call_pall 0x81". Since __builtin_trap () is a
noreturn function, no other instructions are emitted after call_pal.

However, when call_pal insn is executed, alpha updates program counter
before the signal is raised. As call_pal was the last insn, updated PC
points outside of the function boundaries, and backtrace (and gdb,
FWIW) failed to found enclosing function.

The solution is to emit nop after call_pal insn, so PC will always
remain between function boundaries.

Attached patch implements dumpregs function for alpha.

Bootstrapped and regression tested on alphaev68-linux-gnu, and also
checked that gdb and dumpregs produce the same register layout and
values.

Using both pathces, I get:

$ ./testprog Breakpoint
SIGTRAP: trace trap
PC=2199039251764 m=0 sigcode=0

goroutine 1 [running]:
runtime.Breakpoint
/space/homedirs/uros/gcc-svn/trunk/libgo/go/runtime/proc.go:2862
main.Breakpoint

/space/homedirs/uros/gcc-svn/trunk/libgo/go/runtime/testdata/testprog/deadlock.go:145
main.main

/space/homedirs/uros/gcc-svn/trunk/libgo/go/runtime/testdata/testprog/main.go:34

v0  0x2f41530
t0  0x120023f30
t1  0x20ca328
t2  0x2038000
t3  0x22e7766
t4  0x40
t5  0x4
t6  0x1f
t7  0x3f
s0  0x20001f49fb9
s1  0x10
s2  0xc420004960
s3  0x0
s4  0x200017da6a8
s5  0x0
fp  0x20001f49df0
a0  0x11fffd4f4
a1  0x12000dd6c
a2  0x11fffd4fe
a3  0x1
a4  0x17
a5  0x3
t8  0x202e030
t9  0x1
t10 0x0
t11 0x20001f49b20
ra  0x120006a3c
t12 0x2f41530
at  0x6974f985
gp  0x203e030
sp  0x20001f49df0
pc  0x2f41534

Uros.
Index: runtime/go-signal.c
===
--- runtime/go-signal.c (revision 250853)
+++ runtime/go-signal.c (working copy)
@@ -298,4 +298,45 @@ dumpregs(siginfo_t *info __attribute__((unused)),
  }
  #endif
 #endif
+
+#ifdef __alpha__
+  #ifdef __linux__
+   {
+   mcontext_t *m = &((ucontext_t*)(context))->uc_mcontext;
+
+   runtime_printf("v0  %X\n", m->sc_regs[0]);
+   runtime_printf("t0  %X\n", m->sc_regs[1]);
+   runtime_printf("t1  %X\n", m->sc_regs[2]);
+   runtime_printf("t2  %X\n", m->sc_regs[3]);
+   runtime_printf("t3  %X\n", m->sc_regs[4]);
+   runtime_printf("t4  %X\n", m->sc_regs[5]);
+   runtime_printf("t5  %X\n", m->sc_regs[6]);
+   runtime_printf("t6  %X\n", m->sc_regs[7]);
+   runtime_printf("t7  %X\n", m->sc_regs[8]);
+   runtime_printf("s0  %X\n", m->sc_regs[9]);
+   runtime_printf("s1  %X\n", m->sc_regs[10]);
+   runtime_printf("s2  %X\n", m->sc_regs[11]);
+   runtime_printf("s3  %X\n", m->sc_regs[12]);
+   runtime_printf("s4  %X\n", m->sc_regs[13]);
+   runtime_printf("s5  %X\n", m->sc_regs[14]);
+ 

Re: [GCC][PATCH][mid-end][Version 3] Optimize x * copysign (1.0, y) [Patch (1/2)]

[Richard Biener]

On Thu, 3 Aug 2017, Tamar Christina wrote:

> Hi All,
> 
> this patch implements a optimization rewriting
> 
> x * copysign (1.0, y)
> 
> to:
> 
> x ^ (y & (1 << sign_bit_position))
> 
> 
> This is only done when not honoring signaling NaNs.
> This transormation is done at ssa mult widening time and
> is gated on the a check for the optab "xorsign".
> If the optab is not available then copysign is expanded as normal.
> 
> If the optab exists then the expression is replaced with
> a call to the internal function XORSIGN.
> 
> This patch is a revival of a previous patches
> https://gcc.gnu.org/ml/gcc-patches/2015-10/msg00069.html
> https://gcc.gnu.org/ml/gcc-patches/2017-06/msg00749.html
> 
> Bootstrapped on both aarch64-none-linux-gnu and x86_64 with no issues.
> Regression done on aarch64-none-linux-gnu and no regressions.
> 
> Ok for trunk?

+DEF_INTERNAL_OPTAB_FN (XORSIGN, ECF_CONST, xorsign, binary)
+

please avoid adding spurious vertical space

+is_copysign_call_with_1 (gimple *call)
+{
+  if (!is_gimple_call (call))
+return false;
+
+  enum combined_fn code = gimple_call_combined_fn (call);
+
+  if (code == CFN_LAST)
+return false;
+
+  gcall *c = as_a (call);

A better idiom would be

is_copysign_call_with_1 (gimple *call)
{
  gcall *c = dyn_cast  (call);
  if (! c)
return false;

+  if (TREE_CODE (treeop0) == SSA_NAME && TREE_CODE (treeop1) == SSA_NAME)
+{
+  gimple *call0 = SSA_NAME_DEF_STMT (treeop0);
+  if (!is_copysign_call_with_1 (call0))
+   {
+ /* IPA sometimes inlines and then extracts the function again,
+resulting in an incorrect order, so check both ways.  */
+ call0 = SSA_NAME_DEF_STMT (treeop1);

It can happen in both order dependent on SSA name versioning so the
comment is misleading - please drop it.

+   gcall *call_stmt
+ = gimple_build_call_internal (IFN_XORSIGN, 2, treeop1,treeop0);

space before treeop0 missing.

+   gimple_set_lhs (call_stmt, lhs);
+   gimple_set_location (call_stmt, loc);
+   gsi_insert_after (gsi, call_stmt, GSI_SAME_STMT);
+   return true;

please do gsi_replace (gsi, call_stmt, true) instead of inserting after
the original stmt and ...

@@ -4122,6 +4212,11 @@ pass_optimize_widening_mul::execute (function *fun)
  release_defs (stmt);
  continue;
}
+if (convert_expand_mult_copysign (stmt, &gsi))
+   {
+ gsi_remove (&gsi, true);
+ continue;
+   }
  break;

handle this like convert_mult_to_widen, thus

  switch (code)
{
case MULT_EXPR:
  if (!convert_mult_to_widen (stmt, &gsi)
  && !convert_expand_mult_copysign (stmt, &gsi)
  && convert_mult_to_fma (stmt,
  gimple_assign_rhs1 (stmt),
  gimple_assign_rhs2 (stmt)))

given you likely do not want x * copysign (1, y) + z to be transformed
into FMA but still use xorsign?

I am also eventually missing a single-use check on the copysign
result.  If the result of copysign is used in multiple places
do we want to keep the multiply or is the xorsign much cheaper?
The only eventual disadvantage would be higher register pressure
if y and copysing (1, y) were not live at the same time before.

Thanks,
Richard.


> gcc/
> 2017-08-03  Tamar Christina  
>   Andrew Pinski 
> 
>   PR middle-end/19706
>   * internal-fn.def (XORSIGN): New.
>   * optabs.def (xorsign_optab): New.
>   * tree-ssa-math-opts.c (is_copysign_call_with_1): New.
>   (convert_expand_mult_copysign): New.
>   (pass_optimize_widening_mul::execute): Call 
> convert_expand_mult_copysign.
> 
> 

-- 
Richard Biener 
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 
21284 (AG Nuernberg)

Re: [PATCH][RFC] Make expansion of balanced binary trees of switches on tree level.

[Martin Liška]

On 08/03/2017 02:52 PM, Steven Bosscher wrote:
> On Wed, Aug 2, 2017 at 1:20 PM, Martin Liška wrote:
>> Hello.
>>
>> After some discussions with Honza, I've decided to convert current code in 
>> stmt.c that
>> is responsible for switch expansion. More precisely, I would like to convert 
>> the code
>> to expand gswitch statements on tree level. Currently the newly created pass 
>> is executed
>> at the end of tree optimizations.
> 
> Hah, something I promissed myself (and others) to do years ago! Thanks
> thanks thanks! :-)

Hi.

Yes, it's very interesting topic to work on. Well, if you have sparse time, 
help will be
highly appreciated ;)

> 
> The end of the gimple optimizations is seems late for the lowering.
> Before there were gimple optimizations, switch lowering was part of
> the first compiler pass (to generate RTL from the front end) *before*
> all code transformation passes ("optimizations" ;-). Because the
> lowering of switch statements was rewritten as a gimple lowering pass,
> it now runs *after* optimizations. But to be honest, I can't think of
> any optimization opportunities exposed by lowering earlier than at the
> end of gimple optimizations. Years ago there was some RTL jump
> threading still done after lowering of small switch statements, but I
> can't find the related PRs anymore.
> 
> 
>> My plan for future is to inspire in [1] and come up with some more 
>> sophisticated switch
>> expansions. For that I've been working on a paper where I'll summarize 
>> statistics based
>> on what I've collected in openSUSE distribution with specially instrumented 
>> GCC. If I'll be
>> happy I can also fit in to schedule of this year's Cauldron with a talk.
> 
> Sayle's paper is a good starting point. Also interesting:

Yes, I read that.

> 
> http://llvm.org/devmtg/2017-02-04/Efficient-clustering-of-case-statements-for-indirect-branch-prediction.pdf
> About adjusting the size of jump tables to the capabilities of the CPU
> branch predictor. Mixed results but something to consider.

I've also considered to play with jump tables and their sizes. Thanks for the 
article.

> 
> Kannan & Proebsting, "CORRECTION TO ‘PRODUCING GOOD CODE FOR THE CASE
> STATEMENT’"
> About finding "clusers" of given density within the target values of
> the switch statement. The clustering algorithm as presented is N^2 in
> the number of case labels, but the idea is interesting and a
> simplified, cheaper approach may be possible. This is already used in
> LLVM, it seems.

I'll take a look at LLVM as I'm unable to find PDF of the Kannan's article :/

> 
> The real challenge will be to figure out how to pick from all these
> different approaches the right ones to lower a single switch
> statement.

Exactly, that's why I started with porting of the balanced decision tree to 
tree level
and I've been collecting statistics. That should help us what would be 
beneficial and
what's more nice-to-have stuff.

Martin

> 
> Ciao!
> Steven
> 

Re: [PATCH][RFC] Make expansion of balanced binary trees of switches on tree level.

[Martin Liška]

On 08/03/2017 03:27 PM, Steven Bosscher wrote:
> On Thu, Aug 3, 2017 at 2:56 PM, Richard Biener wrote:
>> I think the main reason for not doing it early is the benefit is small
>> (unless it is GIMPLE optimizations triggering)
> 
> Agree.
> 
>> and we can't get rid of
>> switches completely because we eventually have to support casei RTL 
>> expansion.
>> (and no, computed goto with an array of label addresses at GIMPLE is really
>> too ugly ;))
> 
> What I would have done, is lower all gswitch statements that are to be
> lowered to something other than a tablejump.
> So by the time you get to RTL expansion, all remaining gswitch
> statements would be tablejump or casesi.

That exactly my plan :)

> 
> Ciao!
> Steven
> 

[PATCH] Simplify pow with constant

[Wilco Dijkstra]

This patch simplifies pow (C, x) into exp (x * C1), where C1 = log (C).
Do this only for fast-math as accuracy is reduced.  This is much faster
since pow is more complex than exp - with a current GLIBC the speedup
is more than 7 times for this transformation.

ChangeLog:
2017-08-04  Wilco Dijkstra  

* match.pd: Add pow (C, x) simplification.

--

diff --git a/gcc/match.pd b/gcc/match.pd
index 
e98db52af84946cf579c6434e06d450713a47162..96486aa1f512fe32d85a1de95c46523263ea1b6d
 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -3548,6 +3548,14 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
(logs (pows @0 @1))
(mult @1 (logs @0
 
+ /* pow(C,x) -> exp(log(C)*x).  */
+ (for pows (POW)
+  exps (EXP)
+  logs (LOG)
+  (simplify
+   (pows REAL_CST@0 @1)
+   (exps (mult (logs @0) @1
+
  (for sqrts (SQRT)
   cbrts (CBRT)
   pows (POW)

[patch, gcc doc + fortran] Make -Ofast honor -fmax-stack-var-size

[Thomas Koenig]

Hello world,

the attached patch makes -Ofast honor -fmax-stack-var-size, and adjusts
the documentation in the gcc and fortran directories accordingly.
This is done to alleviate PR 68829, to make it possible to run -Ofast
with less stack usage.  I have also taken the opportunity to make
it a bit clearer what -fstack-arrays actually does.

I intend to close PR 68829 once this is committed.

Regression-tested, plus doc changes tested with "make pdf" and
"make dvi".

OK for trunk for

- the fortran subdirectory changes
- the gcc doc changes (if the one above is approved)

?

Regards

Thomas

Re: [PATCH][2/2] early LTO debug, main part

[Richard Biener]

On Thu, 3 Aug 2017, Jason Merrill wrote:

> On Thu, Aug 3, 2017 at 6:51 AM, Richard Biener  wrote:
> > On Wed, 2 Aug 2017, Jason Merrill wrote:
> >
> >> On Wed, Aug 2, 2017 at 6:35 AM, Richard Biener  wrote:
> >> > On Wed, 2 Aug 2017, Jason Merrill wrote:
> >> >
> >> >> On 05/19/2017 06:42 AM, Richard Biener wrote:
> >> >> > + /* ???  In some cases the C++ FE (at least) fails to
> >> >> > +set DECL_CONTEXT properly.  Simply globalize stuff
> >> >> > +in this case.  For example
> >> >> > +__dso_handle created via iostream line 74 col 25.  */
> >> >> > + parent = comp_unit_die ();
> >> >>
> >> >> I've now fixed __dso_handle, so that can be removed from the comment, 
> >> >> but it
> >> >> still makes sense to have this fall-back for the (permitted) case of 
> >> >> null
> >> >> DECL_CONTEXT.
> >> >
> >> > Adjusted the comment.
> >> >
> >> >> > +   /* ???  LANG issue - DW_TAG_module for fortran.  Either look
> >> >> > +at the input language (if we have enough DECL_CONTEXT to follow)
> >> >> > +or use a bit in tree_decl_with_vis to record the distinction.  */
> >> >>
> >> >> Sure, you should be able to look at TRANSLATION_UNIT_LANGUAGE.
> >> >
> >> > Yeah, the comment says we might be able to walk DECL_CONTEXT up to
> >> > a TRANSLATION_UNIT_DECL.  I've amended is_fortran similar to as I
> >> > amended is_cxx, providing an overload for a decl, factoring out common
> >> > code.  So this is now if (is_fortran (decl)) ... = new_die
> >> > (DW_TAG_module,...).
> >> >
> >> >> > ! /* ???  We cannot unconditionally output die_offset if
> >> >> > !non-zero - at least -feliminate-dwarf2-dups will
> >> >> > !create references to those DIEs via symbols.  And we
> >> >> > !do not clear its DIE offset after outputting it
> >> >> > !(and the label refers to the actual DIEs, not the
> >> >> > !DWARF CU unit header which is when using label + 
> >> >> > offset
> >> >> > !would be the correct thing to do).
> >> >>
> >> >> As in our previous discussion, I think -feliminate-dwarf2-dups can go 
> >> >> away
> >> >> now.  But this is a more general issue: die_offset has meant the offset 
> >> >> from
> >> >> the beginning of the CU, but if with_offset is set it means an offset 
> >> >> from
> >> >> die_symbol.  Since with_offset changes the meaning of die_symbol and
> >> >> die_offset, having different code here depending on that flag makes 
> >> >> sense.
> >> >>
> >> >> It seems likely that when -fEDD goes away, we will never again want to 
> >> >> do
> >> >> direct symbolic references to DIEs, in which case we could drop the 
> >> >> current
> >> >> meaning of die_symbol, and so we wouldn't need the with_offset flag.
> >> >
> >> > Yeah, I've been playing with a patch to remove -fEDD but it has conflicts
> >> > with the early LTO debug work and thus I wanted to postpone it until
> >> > after that goes in to avoid further churn.  I hope that's fine, it's
> >> > sth I'll tackle soon after this patch lands on trunk.
> >>
> >> Sure.
> >>
> >> >> > !   /* Don't output the symbol twice.  For LTO we want the label
> >> >> > !  on the section beginning, not on the actual DIE.  */
> >> >> > !   && (!flag_generate_lto
> >> >> > ! || die->die_tag != DW_TAG_compile_unit))
> >> >>
> >> >> I think this check should just be !with_offset; if that flag is set the 
> >> >> DIE
> >> >> doesn't actually have its own symbol.
> >> >
> >> > with_offset is set only during LTRANS phase for the DIEs refering to
> >> > the early DIEs via the CU label.  But the above is guarding the
> >> > early phase when we do not want to output that CU label twice.
> >> >
> >> > Can we revisit this check when -fEDD has gone away?
> >>
> >> Yes.
> >>
> >> >> > !   if (old_die
> >> >> > ! && (c = get_AT_ref (old_die, DW_AT_abstract_origin))
> >> >> > ! /* ???  In LTO all origin DIEs still refer to the early
> >> >> > !debug copy.  Detect that.  */
> >> >> > ! && get_AT (c, DW_AT_inline))
> >> >> ...
> >> >> > !   /* "Unwrap" the decls DIE which we put in the imported unit 
> >> >> > context.
> >> >> > !   ???  If we finish dwarf2out_function_decl refactoring we 
> >> >> > can
> >> >> > ! do this in a better way from the start and only lazily emit
> >> >> > ! the early DIE references.  */
> >> >>
> >> >> It seems like in gen_subprogram_die you deliberately avoid reusing the 
> >> >> DIE
> >> >> from dwarf2out_register_external_die (since it doesn't have 
> >> >> DW_AT_inline), and
> >> >> then in add_abstract_origin_attribute you need to look through that 
> >> >> redundant
> >> >> die.  Why not reuse it?
> >> >
> >> > What we're doing here is dealing with the case of an inlined
> >> > instance which is adjusted to point back to the early debug copy
> >> > directly than to the wrapping DIE (supposed to eventually contain
> >> > the concrete instance).
> >>

[PATCH][AArch64] Use gen_frame_mem for callee-saves

[Wilco Dijkstra]

The frame code uses a mixture of gen_rtx_MEM and gen_frame_mem for
callee-saves.  Callee-saves never alias with local variables, so using
gen_frame_mem is correct. Minor cleanup - no change in code.

OK for commit?

ChangeLog:
2017-08-04  Wilco Dijkstra  

gcc/
* config/aarch64/aarch64.c (aarch64_pushwb_single_reg):
Use gen_frame_mem.
(aarch64_pop_regs): Likewise.
(aarch64_gen_load_pair): Likewise.
(aarch64_save_callee_saves): Likewise.
(aarch64_restore_callee_saves): Likewise.
--

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 
f7cdcaf5a759ee1eb90a4a370d9a3625f7bdc4dd..e5c6c1ca65269a209e893729ce3230f70bd4e808
 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -3072,7 +3072,7 @@ aarch64_pushwb_single_reg (machine_mode mode, unsigned 
regno,
   reg = gen_rtx_REG (mode, regno);
   mem = gen_rtx_PRE_MODIFY (Pmode, base_rtx,
plus_constant (Pmode, base_rtx, -adjustment));
-  mem = gen_rtx_MEM (mode, mem);
+  mem = gen_frame_mem (mode, mem);
 
   insn = emit_move_insn (mem, reg);
   RTX_FRAME_RELATED_P (insn) = 1;
@@ -3160,7 +3160,7 @@ aarch64_pop_regs (unsigned regno1, unsigned regno2, 
HOST_WIDE_INT adjustment,
 {
   rtx mem = plus_constant (Pmode, stack_pointer_rtx, adjustment);
   mem = gen_rtx_POST_MODIFY (Pmode, stack_pointer_rtx, mem);
-  emit_move_insn (reg1, gen_rtx_MEM (mode, mem));
+  emit_move_insn (reg1, gen_frame_mem (mode, mem));
 }
   else
 {
@@ -3236,8 +3236,6 @@ aarch64_save_callee_saves (machine_mode mode, 
HOST_WIDE_INT start_offset,
   unsigned start, unsigned limit, bool skip_wb)
 {
   rtx_insn *insn;
-  rtx (*gen_mem_ref) (machine_mode, rtx) = (frame_pointer_needed
-? gen_frame_mem : gen_rtx_MEM);
   unsigned regno;
   unsigned regno2;
 
@@ -3258,8 +3256,8 @@ aarch64_save_callee_saves (machine_mode mode, 
HOST_WIDE_INT start_offset,
 
   reg = gen_rtx_REG (mode, regno);
   offset = start_offset + cfun->machine->frame.reg_offset[regno];
-  mem = gen_mem_ref (mode, plus_constant (Pmode, stack_pointer_rtx,
- offset));
+  mem = gen_frame_mem (mode, plus_constant (Pmode, stack_pointer_rtx,
+   offset));
 
   regno2 = aarch64_next_callee_save (regno + 1, limit);
 
@@ -3273,8 +3271,8 @@ aarch64_save_callee_saves (machine_mode mode, 
HOST_WIDE_INT start_offset,
  rtx mem2;
 
  offset = start_offset + cfun->machine->frame.reg_offset[regno2];
- mem2 = gen_mem_ref (mode, plus_constant (Pmode, stack_pointer_rtx,
-  offset));
+ mem2 = gen_frame_mem (mode, plus_constant (Pmode, stack_pointer_rtx,
+offset));
  insn = emit_insn (aarch64_gen_store_pair (mode, mem, reg, mem2,
reg2));
 
@@ -3303,8 +3301,6 @@ aarch64_restore_callee_saves (machine_mode mode,
  unsigned limit, bool skip_wb, rtx *cfi_ops)
 {
   rtx base_rtx = stack_pointer_rtx;
-  rtx (*gen_mem_ref) (machine_mode, rtx) = (frame_pointer_needed
-? gen_frame_mem : gen_rtx_MEM);
   unsigned regno;
   unsigned regno2;
   HOST_WIDE_INT offset;
@@ -3325,7 +3321,7 @@ aarch64_restore_callee_saves (machine_mode mode,
 
   reg = gen_rtx_REG (mode, regno);
   offset = start_offset + cfun->machine->frame.reg_offset[regno];
-  mem = gen_mem_ref (mode, plus_constant (Pmode, base_rtx, offset));
+  mem = gen_frame_mem (mode, plus_constant (Pmode, base_rtx, offset));
 
   regno2 = aarch64_next_callee_save (regno + 1, limit);
 
@@ -3338,7 +3334,7 @@ aarch64_restore_callee_saves (machine_mode mode,
  rtx mem2;
 
  offset = start_offset + cfun->machine->frame.reg_offset[regno2];
- mem2 = gen_mem_ref (mode, plus_constant (Pmode, base_rtx, offset));
+ mem2 = gen_frame_mem (mode, plus_constant (Pmode, base_rtx, offset));
  emit_insn (aarch64_gen_load_pair (mode, reg, mem, reg2, mem2));
 
  *cfi_ops = alloc_reg_note (REG_CFA_RESTORE, reg2, *cfi_ops);

Re: [PATCH][1/2] Early LTO debug, simple-object part

[Richard Biener]

On Fri, 28 Jul 2017, Jason Merrill wrote:

> On 07/28/2017 05:55 PM, Jason Merrill wrote:
> > On Fri, Jul 28, 2017 at 8:54 AM, Richard Biener  wrote:
> > > On Tue, 4 Jul 2017, Richard Biener wrote:
> > > 
> > > > On Tue, 20 Jun 2017, Richard Biener wrote:
> > > > 
> > > > > On Wed, 7 Jun 2017, Richard Biener wrote:
> > > > > 
> > > > > > On Fri, 19 May 2017, Richard Biener wrote:
> > > > > > 
> > > > > > > 
> > > > > > > This is a repost (unchanged) of the simple-object ELF support for
> > > > > > > early LTO debug transfer from IL object to a separate debug-only
> > > > > > > object
> > > > > > > file.
> > > > > > > 
> > > > > > > Bootstrapped and tested on x86_64-unknown-linux-gnu.
> > > > > > 
> > > > > > Ping.
> > > > > 
> > > > > Ping^2.
> > > > 
> > > > Ping^3.
> > > 
> > > Ping^4.  Adding some more global reviewers to CC.
> > 
> > Looking at it now, sorry for the delay.
> 
> Actually, the simple-object stuff is more Ian's area.  Looking at the other
> part.

Ian, ping -- we're through with the other part
(https://gcc.gnu.org/ml/gcc-patches/2017-08/msg00374.html).  The
simple-object part is unchanged at

https://gcc.gnu.org/ml/gcc-patches/2017-05/msg01541.html

nearly unchanged from the post last year (which also includes 
some description):

https://gcc.gnu.org/ml/gcc-patches/2016-09/msg01292.html

Thanks,
Richard.

[PATCH][AArch64] Introduce emit_frame_chain

[Wilco Dijkstra]

The current frame code combines the separate concepts of a frame chain
(saving old FP,LR in a record and pointing new FP to it) and a frame
pointer used to access locals.  Add emit_frame_chain to the aarch64_frame
descriptor and use it in the prolog and epilog code.  For now just
initialize it as before, so generated code is identical.

Also correctly set EXIT_IGNORE_STACK.  The current AArch64 epilog code 
restores SP from FP if alloca is used.  If a frame pointer is used but
there is no alloca, SP must remain valid for the epilog to work correctly.

ChangeLog:
2017-08-03  Wilco Dijkstra  

gcc/
* config/aarch64/aarch64.h (EXIT_IGNORE_STACK): Set if alloca is used.
(aarch64_frame): Add emit_frame_chain boolean.
* config/aarch64/aarch64.c (aarch64_frame_pointer_required)
Move eh_return case to aarch64_layout_frame.
(aarch64_layout_frame): Initialize emit_frame_chain.
(aarch64_expand_prologue): Use emit_frame_chain.

--
diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
index 
82d270e575ca40e9786de6a4e27408a32bc09be7..ee23944b191dd17339f42ce8ea7ff9733357c3d8
 100644
--- a/gcc/config/aarch64/aarch64.h
+++ b/gcc/config/aarch64/aarch64.h
@@ -346,9 +346,9 @@ extern unsigned aarch64_architecture_version;
   (epilogue_completed && (REGNO) == LR_REGNUM)
 
 /* EXIT_IGNORE_STACK should be nonzero if, when returning from a function,
-   the stack pointer does not matter.  The value is tested only in
-   functions that have frame pointers.  */
-#define EXIT_IGNORE_STACK  1
+   the stack pointer does not matter.  This is only true if the function
+   uses alloca.  */
+#define EXIT_IGNORE_STACK  (cfun->calls_alloca)
 
 #define STATIC_CHAIN_REGNUMR18_REGNUM
 #define HARD_FRAME_POINTER_REGNUM  R29_REGNUM
@@ -604,6 +604,9 @@ struct GTY (()) aarch64_frame
   /* The size of the stack adjustment after saving callee-saves.  */
   HOST_WIDE_INT final_adjust;
 
+  /* Store FP,LR and setup a frame pointer.  */
+  bool emit_frame_chain;
+
   unsigned wb_candidate1;
   unsigned wb_candidate2;
 
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 
e5c6c1ca65269a209e893729ce3230f70bd4e808..0c355506e2a1605d24d1e89934a063e5f333eebf
 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -2847,10 +2847,6 @@ aarch64_frame_pointer_required (void)
   && (!crtl->is_leaf || df_regs_ever_live_p (LR_REGNUM)))
 return true;
 
-  /* Force a frame pointer for EH returns so the return address is at FP+8.  */
-  if (crtl->calls_eh_return)
-return true;
-
   return false;
 }
 
@@ -2866,6 +2862,10 @@ aarch64_layout_frame (void)
   if (reload_completed && cfun->machine->frame.laid_out)
 return;
 
+  /* Force a frame chain for EH returns so the return address is at FP+8.  */
+  cfun->machine->frame.emit_frame_chain
+= frame_pointer_needed || crtl->calls_eh_return;
+
 #define SLOT_NOT_REQUIRED (-2)
 #define SLOT_REQUIRED (-1)
 
@@ -2900,7 +2900,7 @@ aarch64_layout_frame (void)
last_fp_reg = regno;
   }
 
-  if (frame_pointer_needed)
+  if (cfun->machine->frame.emit_frame_chain)
 {
   /* FP and LR are placed in the linkage record.  */
   cfun->machine->frame.reg_offset[R29_REGNUM] = 0;
@@ -3639,6 +3639,7 @@ aarch64_expand_prologue (void)
   HOST_WIDE_INT callee_offset = cfun->machine->frame.callee_offset;
   unsigned reg1 = cfun->machine->frame.wb_candidate1;
   unsigned reg2 = cfun->machine->frame.wb_candidate2;
+  bool emit_frame_chain = cfun->machine->frame.emit_frame_chain;
   rtx_insn *insn;
 
   /* Sign return address for functions.  */
@@ -3669,7 +3670,7 @@ aarch64_expand_prologue (void)
   if (callee_adjust != 0)
 aarch64_push_regs (reg1, reg2, callee_adjust);
 
-  if (frame_pointer_needed)
+  if (emit_frame_chain)
 {
   if (callee_adjust == 0)
aarch64_save_callee_saves (DImode, callee_offset, R29_REGNUM,
@@ -3677,12 +3678,12 @@ aarch64_expand_prologue (void)
   insn = emit_insn (gen_add3_insn (hard_frame_pointer_rtx,
   stack_pointer_rtx,
   GEN_INT (callee_offset)));
-  RTX_FRAME_RELATED_P (insn) = 1;
+  RTX_FRAME_RELATED_P (insn) = frame_pointer_needed;
   emit_insn (gen_stack_tie (stack_pointer_rtx, hard_frame_pointer_rtx));
 }
 
   aarch64_save_callee_saves (DImode, callee_offset, R0_REGNUM, R30_REGNUM,
-callee_adjust != 0 || frame_pointer_needed);
+callee_adjust != 0 || emit_frame_chain);
   aarch64_save_callee_saves (DFmode, callee_offset, V0_REGNUM, V31_REGNUM,
 callee_adjust != 0 || frame_pointer_needed);
   aarch64_sub_sp (IP1_REGNUM, final_adjust, !frame_pointer_needed);

Re: [PATCH] Simplify pow with constant

[Alexander Monakov]

On Fri, 4 Aug 2017, Wilco Dijkstra wrote:
> This patch simplifies pow (C, x) into exp (x * C1), where C1 = log (C).

I don't think you can do that for non-positive C.

> Do this only for fast-math as accuracy is reduced.  This is much faster
> since pow is more complex than exp - with a current GLIBC the speedup
> is more than 7 times for this transformation.

Is it bound to be so on future glibc revisions and non-glibc platforms?

Alexander

Re: libgo patch committed: Fix signal counting for glibc 2.26

[Ian Lance Taylor]

On Fri, Aug 4, 2017 at 12:22 AM, Richard Biener
 wrote:
> On Thu, Aug 3, 2017 at 8:11 PM, Ian Lance Taylor  wrote:
>> This patch to libgo changes the mksigtab script to recognize the glibc
>> 2.26 NSIG expression.  Bootstrapped and ran Go testsuite on
>> x86_64-pc-linux-gnu.  Committed to mainline.  Will commit to GCC 7
>> branch when it reopens.  This fixes GCC PR 81617 and
>> https://golang.org/issue/21147.
>
> The patch looks pretty safe as it only changes things when nsig is empty
> so can you install it on the branch now so that 7.2 will build Go fine with
> glibc 2.26?

Done.

Ian

Re: [PING^4][PATCH v2] Generate reproducible output independently of the build-path

[Matthias Klose]

On 03.08.2017 12:05, Jeff Law wrote:
> On 08/02/2017 08:06 PM, Ximin Luo wrote:
>> Jeff Law:
>>> On 07/21/2017 10:15 AM, Ximin Luo wrote:
 (Please keep me on CC, I am not subscribed)


 Proposal
 

 This patch series adds a new environment variable BUILD_PATH_PREFIX_MAP. 
 When
 this is set, GCC will treat this as extra implicit 
 "-fdebug-prefix-map=$value"
 command-line arguments that precede any explicit ones. This makes the final
 binary output reproducible, and also hides the unreproducible value (the 
 source
 path prefixes) from CFLAGS et. al. which many build tools (understandably)
 embed as-is into their build output.
>>> I'd *really* avoid doing this with magic environment variables.  Make it
>>> a first class option to the compiler.  Yes, it means projects that want
>>> this behavior have to arrange to pass that flag to their compiler, but
>>> IMHO that's much preferred over environment variables.
>>>
>>> Jeff
>>>
>>
>> Hi Jeff,
>>
>> If by "first class option" you meant a command-line flag, GCC *already has* 
>> that (-fdebug-prefix-map) and it wasn't enough to achieve reproducibility in 
>> many cases we tested. dpkg-buildflags actually already adds these flags to 
>> CFLAGS CXXFLAGS etc on Debian. However, with this patch using the 
>> environment variable, we are able to reproduce 1800 more packages out of 
>> 26000.Then take what you've done with the environment variable and instead
> implement it on top of a switch.  An environment variable is absolutely
> the wrong thing to do here.
> 
>>
>> GCC already supports a similar environment variable SOURCE_DATE_EPOCH, which 
>> was accepted about 2 years ago in a patch written by one of our GSoC 
>> students. We are not planning any more environment variables like this, and 
>> are committed to fixing other sources of non-determinism by patching the 
>> relevant build scripts.
> I would have rejected that as well :-)  One of the few times I would
> have disagreed with Bernd.

You can argue that gcc has command line options to set these, but most build
systems can be influenced by well documented environment variables like CFLAGS,
CXXFLAGS, LDFLAGS, so adding one more variable like SOURCE_DATE_EPOCH makes
sense, considering that many tools dealing with timestamps don't even have
command line options to do these (and there it's not just about compilers).

BUILD_PATH_PREFIX_MAP might be passed as well in CFLAGS / CXXFLAGS, but there
are build systems that even ignore these environment variables, and it's common
to require the use of environment variables for distributions, like

http://pkgs.fedoraproject.org/cgit/rpms/gcc.git/tree/gcc7-foffload-default.patch

If changes like that are not allowed to go upstream, where do you want to keep
these instead, and keeping these in sync across distributions?

Matthias

[PATCH][AArch64] Remove aarch64_frame_pointer_required

[Wilco Dijkstra]

To implement -fomit-leaf-frame-pointer, there are 2 places where we need
to check whether we have to use a frame chain (since register allocation
may allocate LR in a leaf function that omits the frame pointer, but if
LR is spilled we must emit a frame chain).  To simplify this do not force
frame_pointer_needed via aarch64_frame_pointer_required, but enable the
frame chain in aarch64_layout_frame.  Now aarch64_frame_pointer_required
can be removed and aarch64_can_eliminate is simplified.

OK for commit?

ChangeLog:
2017-08-03  Wilco Dijkstra  

gcc/
* config/aarch64/aarch64.c (aarch64_frame_pointer_required)
Remove.
(aarch64_layout_frame): Initialise emit_frame_chain.
(aarch64_can_eliminate): Remove omit leaf frame pointer code.
(TARGET_FRAME_POINTER_REQUIRED): Remove define.
--

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 
aa71a410106164bb8da808a4b513771d713bb0f0..9bbc9864fd47a4404a80ea0cd5608202e8d0726a
 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -2836,21 +2836,6 @@ aarch64_output_probe_stack_range (rtx reg1, rtx reg2)
   return "";
 }
 
-static bool
-aarch64_frame_pointer_required (void)
-{
-  /* Use the frame pointer if enabled and it is not a leaf function, unless
- leaf frame pointer omission is disabled.  If the frame pointer is enabled,
- force the frame pointer in leaf functions which use LR.  */
-  if (flag_omit_frame_pointer == 2
-  && !(flag_omit_leaf_frame_pointer
-  && crtl->is_leaf
-  && !df_regs_ever_live_p (LR_REGNUM)))
-return true;
-
-  return false;
-}
-
 /* Mark the registers that need to be saved by the callee and calculate
the size of the callee-saved registers area and frame record (both FP
and LR may be omitted).  */
@@ -2867,6 +2852,14 @@ aarch64_layout_frame (void)
   cfun->machine->frame.emit_frame_chain
 = frame_pointer_needed || crtl->calls_eh_return;
 
+  /* Emit a frame chain if the frame pointer is enabled.
+ If -momit-leaf-frame-pointer is used, do not use a frame chain
+ in leaf functions which do not use LR.  */
+  if (flag_omit_frame_pointer == 2
+  && !(flag_omit_leaf_frame_pointer && crtl->is_leaf
+  && !df_regs_ever_live_p (LR_REGNUM)))
+cfun->machine->frame.emit_frame_chain = true;
+
 #define SLOT_NOT_REQUIRED (-2)
 #define SLOT_REQUIRED (-1)
 
@@ -5884,17 +5877,6 @@ aarch64_can_eliminate (const int from, const int to)
 
   return false;
 }
-  else
-{
-  /* If we decided that we didn't need a leaf frame pointer but then used
-LR in the function, then we'll want a frame pointer after all, so
-prevent this elimination to ensure a frame pointer is used.  */
-  if (to == STACK_POINTER_REGNUM
- && flag_omit_frame_pointer == 2
- && flag_omit_leaf_frame_pointer
- && df_regs_ever_live_p (LR_REGNUM))
-   return false;
-}
 
   return true;
 }
@@ -15385,9 +15367,6 @@ aarch64_run_selftests (void)
 #undef TARGET_FUNCTION_VALUE_REGNO_P
 #define TARGET_FUNCTION_VALUE_REGNO_P aarch64_function_value_regno_p
 
-#undef TARGET_FRAME_POINTER_REQUIRED
-#define TARGET_FRAME_POINTER_REQUIRED aarch64_frame_pointer_required
-
 #undef TARGET_GIMPLE_FOLD_BUILTIN
 #define TARGET_GIMPLE_FOLD_BUILTIN aarch64_gimple_fold_builtin
 

Re: [PATCH] Simplify pow with constant

[Richard Biener]

On Fri, Aug 4, 2017 at 2:26 PM, Alexander Monakov  wrote:
> On Fri, 4 Aug 2017, Wilco Dijkstra wrote:
>> This patch simplifies pow (C, x) into exp (x * C1), where C1 = log (C).
>
> I don't think you can do that for non-positive C.

Hmm, the question is also how this interacts with other folders like
sqrt (pow (x, y)) -> pow (|x|, y * 0,5)?  Also we seem to miss
pow (2, x) -> exp2 (x) and pow (10, x) -> pow10/exp10, those may
be a better fit than exp (log (2/10) * x)?  OTOH for fast-math
canonicalization getting rid of exp2/10 and pow10 might be beneficial.

>> Do this only for fast-math as accuracy is reduced.  This is much faster
>> since pow is more complex than exp - with a current GLIBC the speedup
>> is more than 7 times for this transformation.
>
> Is it bound to be so on future glibc revisions and non-glibc platforms?

And how is accuracy affected?  I think the transform is only reasonable
for log (C) being close to e, 2 or 10 (using exp, exp2 or exp10).  Can you
provide an idea on whether there's a systematic error (with glibc) and
how that behaves over the parameter space?

Oh, and what value of C does the benchmark that triggered this have?

Richard.

> Alexander

Re: [PING^4][PATCH v2] Generate reproducible output independently of the build-path

[Jakub Jelinek]

On Fri, Aug 04, 2017 at 08:32:33AM -0400, Matthias Klose wrote:
> >> GCC already supports a similar environment variable SOURCE_DATE_EPOCH, 
> >> which was accepted about 2 years ago in a patch written by one of our GSoC 
> >> students. We are not planning any more environment variables like this, 
> >> and are committed to fixing other sources of non-determinism by patching 
> >> the relevant build scripts.
> > I would have rejected that as well :-)  One of the few times I would
> > have disagreed with Bernd.
> 
> You can argue that gcc has command line options to set these, but most build
> systems can be influenced by well documented environment variables like 
> CFLAGS,
> CXXFLAGS, LDFLAGS, so adding one more variable like SOURCE_DATE_EPOCH makes
> sense, considering that many tools dealing with timestamps don't even have
> command line options to do these (and there it's not just about compilers).

Unlike SOURCE_DATE_EPOCH, the other env vars are autoconf/cmake etc.
related, we really shouldn't be adding more of these.
If some package has messed up build system, you can use
CXX='g++ -fwhatever'
or whatever other way to pass flags you want to the compiler or pick the
compiler you prefer to use.

> BUILD_PATH_PREFIX_MAP might be passed as well in CFLAGS / CXXFLAGS, but there
> are build systems that even ignore these environment variables, and it's 
> common
> to require the use of environment variables for distributions, like
> 
> http://pkgs.fedoraproject.org/cgit/rpms/gcc.git/tree/gcc7-foffload-default.patch

Dunno how this patch is related to the env var vs. command line option
question, the env var in the patch is used to communicate stuff from the
gcc driver to subcommands.  It isn't something users should be using on
their own.

Jakub

Re: [PATCH v2][RFC] Canonize names of attributes.

[Martin Liška]

On 08/02/2017 01:25 PM, Joseph Myers wrote:
> On Thu, 13 Jul 2017, Martin Liška wrote:
> 
>> +/* For a given IDENTIFIER_NODE, strip leading and trailing '_' characters
>> +   so that we have a canonical form of attribute names.  */
>> +
>> +static inline tree
>> +canonicalize_attr_name (tree attr_name)
>> +{
>> +  const size_t l = IDENTIFIER_LENGTH (attr_name);
>> +  const char *s = IDENTIFIER_POINTER (attr_name);
>> +
>> +  if (l > 4 && s[0] == '_')
>> +{
>> +  gcc_checking_assert (s[l - 2] == '_');
>> +  return get_identifier_with_length (s + 2, l - 4);
>> +}
>> +
>> +  return attr_name;
> 
> For this to (a) be correct, (b) not trigger the assertion, there must be a 
> precondition that attr_name either starts and ends with __, or does not 
> start with _, or has length 4 or less.  I don't see anything in the 
> callers to ensure this precondition holds, so that, for example, 
> __attribute__ ((_foobar)) does not trigger the assertion, and 
> __attribute__ ((_xformat__)) is not wrongly interpreted as a format 
> attribute (and similarly for attribute arguments when those are 
> canonicalized).

Thanks for the review. I've updated to to canonicalize just __.+__.
I added test-case for that: gcc/testsuite/gcc.dg/Wattributes-5.c.

> 
>> +/* Compare attribute identifiers ATTR1 and ATTR2 with length ATTR1_LEN and
>> +   ATTR2_LEN.  */
>> +
>> +static inline bool
>> +cmp_attribs (const char *attr1, size_t attr1_len,
>> + const char *attr2, size_t attr2_len)
>> +{
>> +  gcc_checking_assert (attr1_len == 0 || attr1[0] != '_');
>> +  gcc_checking_assert (attr2_len == 0 || attr2[0] != '_');

And I removed these asserts.

Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.

Ready to be installed?
Martin

> 
> Of course, once you only canonicalize attributes that both start and end 
> with __, you have the possibility of a canonicalized attribute name that 
> still starts with _, so can't assert on it here (unless this is only ever 
> called with an attribute that is already known to be valid).  (And of 
> course there are cases such as __attribute__((__)) that starts and ends 
> with __ but is too short to canonicalize, etc., which arise even with the 
> above canonicalize_attr_name.)
> 

>From 06c710c9eb74212d78dd82c5fcb36a86a368a086 Mon Sep 17 00:00:00 2001
From: marxin 
Date: Thu, 8 Jun 2017 10:23:25 +0200
Subject: [PATCH 1/2] Canonicalize names of attributes.

gcc/ChangeLog:

2017-07-12  Martin Liska  

	* attribs.h (canonicalize_attr_name): New function.
	(cmp_attribs): Move from c-format.c and adjusted.
	(is_attribute_p): Moved from tree.h.
	* tree-inline.c: Add new includes.
	* tree.c (cmp_attrib_identifiers): Use cmp_attribs.
	(private_is_attribute_p): Remove.
	(private_lookup_attribute): Likewise.
	(private_lookup_attribute_by_prefix): Simplify.
	(remove_attribute): Use is_attribute_p.
	* tree.h: Remove removed declarations.

gcc/c-family/ChangeLog:

2017-07-12  Martin Liska  

	* array-notation-common.c: Add new includes.
	* c-format.c( handle_format_attribute): Canonicalize a format
	function name.
	* c-lex.c (c_common_has_attribute): Canonicalize name of an
	attribute.
	* c-pretty-print.c: Add new include.

gcc/cp/ChangeLog:

2017-07-12  Martin Liska  

	* parser.c (cp_parser_gnu_attribute_list): Canonicalize name of an
	attribute.
	(cp_parser_std_attribute): Likewise.
	* tree.c: Add new include.

gcc/c/ChangeLog:

2017-07-12  Martin Liska  

	* c-parser.c (c_parser_attributes): Canonicalize name of an
	attribute.
gcc/go/ChangeLog:

2017-06-29  Martin Liska  

	* go-gcc.cc (Gcc_backend::function): Look up for no_split_stack
	and not __no_split_stack__.

gcc/testsuite/ChangeLog:

2017-06-29  Martin Liska  

	* g++.dg/cpp0x/pr65558.C: Update scanned pattern.
	* gcc.dg/parm-impl-decl-1.c: Likewise.
	* gcc.dg/parm-impl-decl-3.c: Likewise.
	* gcc.dg/Wattributes-5.c: New test.
---
 gcc/attribs.h   |  43 +
 gcc/c-family/array-notation-common.c|   2 +
 gcc/c-family/c-format.c |  24 ++--
 gcc/c-family/c-lex.c|   1 +
 gcc/c-family/c-pretty-print.c   |   1 +
 gcc/c/c-parser.c|   3 +
 gcc/cp/parser.c |   6 +-
 gcc/cp/tree.c   |   1 +
 gcc/go/go-gcc.cc|   2 +-
 gcc/testsuite/g++.dg/cpp0x/pr65558.C|   2 +-
 gcc/testsuite/gcc.dg/Wattributes-5.c|  13 
 gcc/testsuite/gcc.dg/parm-impl-decl-1.c |   2 +-
 gcc/testsuite/gcc.dg/parm-impl-decl-3.c |   2 +-
 gcc/tree-inline.c   |   3 +-
 gcc/tree.c  | 104 +---
 gcc/tree.h  |  23 +--
 16 files changed, 95 insertions(+), 137 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/Wattributes-5.c

diff --git a/gcc/attribs.h b/gcc/attribs.h
index 7f13332700e..d4a790bb753 100644
--- a/gcc/attribs.h
+++ b/gcc/attribs.h
@@ -47,4 +47,47 @@ ext

Re: gotools patch committed: Test runtime, misc/cgo/{test,testcarchive}

[Ian Lance Taylor]

On Fri, Aug 4, 2017 at 3:54 AM, Uros Bizjak  wrote:
>
> The problem was following:
>
> runtime.Breakpoint comprises only call to __builtin_trap (), which in
> case of alpha maps to "call_pall 0x81". Since __builtin_trap () is a
> noreturn function, no other instructions are emitted after call_pal.
>
> However, when call_pal insn is executed, alpha updates program counter
> before the signal is raised. As call_pal was the last insn, updated PC
> points outside of the function boundaries, and backtrace (and gdb,
> FWIW) failed to found enclosing function.
>
> The solution is to emit nop after call_pal insn, so PC will always
> remain between function boundaries.
>
> Attached patch implements dumpregs function for alpha.

Thanks for analyzing the problem.

I committed the dump-registers patch to mainline.

Ian

Re: [PATCH][AArch64][GCC 6] PR target/79041: Correct -mpc-relative-literal-loads logic in aarch64_classify_symbol

[Yvan Roux]

On 11 July 2017 at 12:26, Yvan Roux  wrote:
> On 3 July 2017 at 12:48, Yvan Roux  wrote:
>> On 27 June 2017 at 13:14, Yvan Roux  wrote:
>>> Hi Wilco
>>>
>>> On 27 June 2017 at 12:53, Wilco Dijkstra  wrote:
 Hi Yvan,

> Here is the backport of Wilco's patch (r237607) along with Kyrill's
> one (r244643, which removed the remaining occurences of
> aarch64_nopcrelative_literal_loads).  To fix the issue the original
> patch has to be modified, to keep aarch64_pcrelative_literal_loads
> test for large models in aarch64_classify_symbol.

 The patch looks good to me, however I can't approve it.
>>>
>>> ok thanks for the review.
>>>
> On trunk and gcc-7-branch the :lo12: relocations are not generated
> because of Wilco's fix for pr78733 (r243456 and 243486), but my
> understanding is that the bug is still present since compiling
> gcc.target/aarch64/pr78733.c with -mcmodel=large brings back the
> :lo12: relocations (I'll submit a patch to add the test back if my
> understanding is correct).

 You're right, eventhough -mpc-relative-literal-loads doesn't make much 
 sense
 in the large memory model, it seems best to keep the option orthogonal to
 enable the workaround. I've prepared a patch to fix this on trunk/GCC7.
 It also adds a test which we should add to your changes to GCC6 too.
>>>
>>> ok, I think it is what kugan's proposed earlier today in:
>>>
>>> https://gcc.gnu.org/ml/gcc-patches/2017-06/msg01967.html
>>>
>>> I agree that -mpc-relative-literal-loads and large memory model
>>> doesn't make much sense, now it is what is used in kernel build
>>> system, but if you handle that in a bigger fix already, that's awesome
>>> :)
>>
>> ping?
>> https://gcc.gnu.org/ml/gcc-patches/2017-06/msg01708.html
>
> ping

ping^3

I can include the new testcase added recently on trunk by Wilco
(gcc.target/aarch64/pr79041-2.c) if needed.

>>> Thanks
>>> Yvan
>>>
 Wilco

Re: [Libgomp, Fortran] Fix canadian cross build

[Yvan Roux]

On 11 July 2017 at 12:25, Yvan Roux  wrote:
> On 3 July 2017 at 11:21, Yvan Roux  wrote:
>> On 23 June 2017 at 15:44, Yvan Roux  wrote:
>>> Hello,
>>>
>>> Fortran parts of libgomp (omp_lib.mod, openacc.mod, etc...) are
>>> missing in a canadian cross build, at least when target gfortran
>>> compiler comes from PATH and not from GFORTRAN_FOR_TARGET.
>>>
>>> Back in 2010, executability test of GFORTRAN was added to fix libgomp
>>> build on cygwin, but when the executable doesn't contain the path,
>>> "test -x" fails and part of the library are not built.
>>>
>>> This patch fixes the issue by using M4 macro AC_PATH_PROG (which
>>> returns the absolute name) instead of AC_CHECK_PROG in the function
>>> defined in config/acx.m4: NCN_STRICT_CHECK_TARGET_TOOLS.  I renamed it
>>> into NCN_STRICT_PATH_TARGET_TOOLS to keep the semantic used in M4.
>>>
>>> Tested by building cross and candian cross toolchain (host:
>>> i686-w64-mingw32) for arm-linux-gnueabihf with issue and with a
>>> complete libgomp.
>>>
>>> ok for trunk ?
>>
>> ping?
>
> ping?

ping

>>
>>> Thanks
>>> Yvan
>>>
>>> config/ChangeLog
>>> 2017-06-23  Yvan Roux  
>>>
>>> * acx.m4 (NCN_STRICT_CHECK_TARGET_TOOLS): Renamed to ...
>>> (NCN_STRICT_PATH_TARGET_TOOLS): ... this.  It reflects the 
>>> replacement
>>> of AC_CHECK_PROG by AC_PATH_PROG to get the absolute name of the
>>> program.
>>> (ACX_CHECK_INSTALLED_TARGET_TOOL): Use renamed function.
>>>
>>> ChangeLog
>>> 2017-06-23  Yvan Roux  
>>>
>>> * configure.ac: Use NCN_STRICT_PATH_TARGET_TOOLS instead of
>>> NCN_STRICT_CHECK_TARGET_TOOLS.
>>> * configure: Regenerate.

Re: [RFC][PATCH] Do refactoring of attribute functions and move them to attribs.[hc].

[Martin Liška]

On 07/14/2017 09:23 AM, Jeff Law wrote:
> On 07/13/2017 07:51 AM, Martin Liška wrote:
>> Hi.
>>
>> It's request for comment where I mechanically moved attribute-related 
>> function to attribs.[hc].
>>
>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
>>
>> Thoughts?
> Seems reasonable.  We don't like to move things around without a good
> reason and I assume you've got something in mind?
> 
> jeff
> 

Thanks. There's updated version (w/ ChangeLog)
of the patch that I'll install right after the first patch will be included.

Martin
>From 140b161f5dab7bba318a233259fbd002abbd0002 Mon Sep 17 00:00:00 2001
From: marxin 
Date: Wed, 12 Jul 2017 13:39:54 +0200
Subject: [PATCH 2/2] Do refactoring of attribute functions and move them to
 attribs.[hc].

gcc/ada/ChangeLog:

2017-08-04  Martin Liska  

	* gcc-interface/trans.c: Include header files.

gcc/objc/ChangeLog:

2017-08-04  Martin Liska  

	* objc-gnu-runtime-abi-01.c: Include header files.
	* objc-next-runtime-abi-01.c: Likewise.
	* objc-next-runtime-abi-02.c: Likewise.

gcc/ChangeLog:

2017-08-04  Martin Liska  

	* asan.c: Include header files.
	* attribs.c (build_decl_attribute_variant): New function moved
	from tree.[ch].
	(build_type_attribute_qual_variant): Likewise.
	(cmp_attrib_identifiers): Likewise.
	(simple_cst_list_equal): Likewise.
	(omp_declare_simd_clauses_equal): Likewise.
	(attribute_value_equal): Likewise.
	(comp_type_attributes): Likewise.
	(build_type_attribute_variant): Likewise.
	(lookup_ident_attribute): Likewise.
	(remove_attribute): Likewise.
	(merge_attributes): Likewise.
	(merge_type_attributes): Likewise.
	(merge_decl_attributes): Likewise.
	(merge_dllimport_decl_attributes): Likewise.
	(handle_dll_attribute): Likewise.
	(attribute_list_equal): Likewise.
	(attribute_list_contained): Likewise.
	* attribs.h (lookup_attribute): New function moved from tree.[ch].
	(lookup_attribute_by_prefix): Likewise.
	* bb-reorder.c: Include header files.
	* builtins.c: Likewise.
	* calls.c: Likewise.
	* cfgexpand.c: Likewise.
	* cgraph.c: Likewise.
	* cgraphunit.c: Likewise.
	* convert.c: Likewise.
	* dwarf2out.c: Likewise.
	* final.c: Likewise.
	* fold-const.c: Likewise.
	* function.c: Likewise.
	* gimple-expr.c: Likewise.
	* gimple-fold.c: Likewise.
	* gimple-pretty-print.c: Likewise.
	* gimple.c: Likewise.
	* gimplify.c: Likewise.
	* hsa-common.c: Likewise.
	* hsa-gen.c: Likewise.
	* internal-fn.c: Likewise.
	* ipa-chkp.c: Likewise.
	* ipa-cp.c: Likewise.
	* ipa-devirt.c: Likewise.
	* ipa-fnsummary.c: Likewise.
	* ipa-inline.c: Likewise.
	* ipa-visibility.c: Likewise.
	* ipa.c: Likewise.
	* lto-cgraph.c: Likewise.
	* omp-expand.c: Likewise.
	* omp-general.c: Likewise.
	* omp-low.c: Likewise.
	* omp-offload.c: Likewise.
	* omp-simd-clone.c: Likewise.
	* opts-global.c: Likewise.
	* passes.c: Likewise.
	* predict.c: Likewise.
	* sancov.c: Likewise.
	* sanopt.c: Likewise.
	* symtab.c: Likewise.
	* toplev.c: Likewise.
	* trans-mem.c: Likewise.
	* tree-chkp.c: Likewise.
	* tree-eh.c: Likewise.
	* tree-into-ssa.c: Likewise.
	* tree-object-size.c: Likewise.
	* tree-parloops.c: Likewise.
	* tree-profile.c: Likewise.
	* tree-ssa-ccp.c: Likewise.
	* tree-ssa-live.c: Likewise.
	* tree-ssa-loop.c: Likewise.
	* tree-ssa-sccvn.c: Likewise.
	* tree-ssa-structalias.c: Likewise.
	* tree-ssa.c: Likewise.
	* tree-streamer-in.c: Likewise.
	* tree-vectorizer.c: Likewise.
	* tree-vrp.c: Likewise.
	* tsan.c: Likewise.
	* ubsan.c: Likewise.
	* varasm.c: Likewise.
	* varpool.c: Likewise.
	* tree.c: Remove functions moved to attribs.[ch].
	* tree.h: Likewise.

gcc/cp/ChangeLog:

2017-08-04  Martin Liska  

	* call.c: Include header files.
	* cp-gimplify.c: Likewise.
	* cp-ubsan.c: Likewise.
	* cvt.c: Likewise.
	* init.c: Likewise.
	* search.c: Likewise.
	* semantics.c: Likewise.
	* typeck.c: Likewise.

gcc/lto/ChangeLog:

2017-08-04  Martin Liska  

	* lto-lang.c: Include header files.
	* lto-symtab.c: Likewise.

gcc/c/ChangeLog:

2017-08-04  Martin Liska  

	* c-convert.c: Include header files.
	* c-typeck.c: Likewise.

gcc/c-family/ChangeLog:

2017-08-04  Martin Liska  

	* c-ada-spec.c: Include header files.
	* c-ubsan.c: Likewise.
	* c-warn.c: Likewise.

gcc/fortran/ChangeLog:

2017-08-04  Martin Liska  

	* trans-types.c: Include header files.
---
 gcc/ada/gcc-interface/trans.c   |   2 +
 gcc/asan.c  |   2 +
 gcc/attribs.c   | 633 +
 gcc/attribs.h   | 113 ++
 gcc/bb-reorder.c|   2 +
 gcc/builtins.c  |   2 +
 gcc/c-family/c-ada-spec.c   |   2 +
 gcc/c-family/c-ubsan.c  |   4 +-
 gcc/c-family/c-warn.c   |   2 +
 gcc/c/c-convert.c   |   2 +
 gcc/c/c-typeck.c|   2 +
 gcc/calls.c |   2 +
 gcc/cfgexpand.c |   2 +
 gcc/cgraph.c|   2 +
 gcc/cgraphunit.c   

Re: [PATCH] PR c++/80287 add new testcase

[Yvan Roux]

On 13 July 2017 at 14:02, Yvan Roux  wrote:
> Hi,
>
> as discussed in the PR, this patch adds a new reduced testcase which
> doesn't rely on c++17 features, this is a prereq to the backport of
> the fix into GCC 6 branch which is impacted by this issue.
>
> Validated on x86, ARM and AArch64 targets.
>
> Ok for trunk ? and maybe on gcc-7-branch ?

ping

> Thanks,
> Yvan
>
> gcc/testsuite
> 2017-07-13  Yvan Roux  
>
> PR c++/80287
> * g++.dg/pr80287.C: New test.

Re: [Patch] Testsuite fixes for failures caused by patch for PR 80925 - loop peeling and alignment

[Rainer Orth]

Richard Biener  writes:

> On Fri, Jul 28, 2017 at 8:22 PM, Steve Ellcey  wrote:
>> On Fri, 2017-07-28 at 09:47 +0200, Richard Biener wrote:
>>> On Fri, Jul 28, 2017 at 12:16 AM, Steve Ellcey  wrote:
>>> >
>>> > Any comments from the power and/or vectorizer folks?
>>> On one side I'm inclined to simplify the testsuite by adding
>>> --param vect-max-peeling-for-alignment=0 in addition to
>>> -fno-vect-cost-model we already pass and override that in the
>>> tests that specifically exercise peeling for alignment (do we have
>>> any?).
>>> OTOH that would remove quite some testing coverage of prologue
>>> peeling.
>>>
>>> So ideally testresults would be clean with both no such --param
>>> and that --param added...
>>>
>>> I think most of the testcases you needed to adjust have nothing
>>> to do with peeling for alignment thus adding this --param just for
>>> those (and simplifying their dump scanning accordingly) is another
>>> pragmatic option.
>>>
>>> Adding yet another target (vect_peel_align) is IMHO not good,
>>> especially as this one depends on cost tuning and not HW
>>> features, so it's impossible(?) to dynamically compute it
>>> with a test compile for example (we _do_ want a clean
>>> vect.exp with any vector HW / tuning switch you add).
>>
>> How about something like the following.  I only fixed two of the tests,
>> I can follow up with more if this approach seems reasonable.  I tested
>> this on aarch64 and x86_64.
>
> Looks good to me.
>
> Richard.
>
>> Steve Ellcey
>> sell...@cavium.com
>>
>>
>> 2017-07-28  Steve Ellcey  
>>
>> PR tree-optimization/80925
>> * gcc.dg/vect/no-section-anchors-vect-69.c: Add
>> --param vect-max-peeling-for-alignment=0 option.
>> Remove unaligned access and peeling checks.
>> * gcc.dg/vect/section-anchors-vect-69.c: Ditto.

This broke the test on Solaris/SPARC:

+FAIL: gcc.dg/vect/no-section-anchors-vect-69.c scan-tree-dump-times vect 
"vectorized 4 loops" 1

both 32 and 64-bit.  The dump now has

no-section-anchors-vect-69.c:39:5: note: vectorized 3 loops in function.

and (compared to the gcc-7 branch)

no-section-anchors-vect-69.c:44:3: note: not vectorized: unsupported unaligned 
store.tmp1[2].a.n[1][2][i_74]

Rainer

-- 
-
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [PATCH 2/3] Matching tokens: C parts (v2)

[David Malcolm]

On Thu, 2017-08-03 at 11:34 -0600, Jeff Law wrote:
> On 08/01/2017 02:21 PM, David Malcolm wrote:
> > Changed in v2:
> > 
> > * Renamed template argument to traits_t; eliminated subclasses,
> > just
> >   using traits struct.
> > * Moved enum constants into struct bodies (string constants can't
> > be
> >   without constexpr, which isn't available in C++98).
> > * Fixed typo.
> > 
> > OK for trunk?
> > 
> > gcc/c/ChangeLog:
> > * c-parser.c (c_parser_error): Rename to...
> > (c_parser_error_richloc): ...this, making static, and adding
> > "richloc" parameter, passing it to the c_parse_error call,
> > rather than calling c_parser_set_source_position_from_token.
> > (c_parser_error): Reintroduce, reimplementing in terms of the
> > above, converting return type from void to bool.
> > (class token_pair): New class.
> > (struct matching_paren_traits): New struct.
> > (matching_parens): New typedef.
> > (struct matching_brace_traits): New struct.
> > (matching_braces): New typedef.
> > (get_matching_symbol): New function.
> > (c_parser_require): Add param MATCHING_LOCATION, using it to
> > highlight matching "opening" tokens for missing "closing"
> > tokens.
> > (c_parser_skip_until_found): Likewise.
> > (c_parser_static_assert_declaration_no_semi): Convert explicit
> > parsing of CPP_OPEN_PAREN and CPP_CLOSE_PAREN to use of
> > class matching_parens, so that the pertinent open parenthesis
> > is
> > highlighted when there are problems locating the close
> > parenthesis.
> > (c_parser_struct_or_union_specifier): Likewise.
> > (c_parser_typeof_specifier): Likewise.
> > (c_parser_alignas_specifier): Likewise.
> > (c_parser_simple_asm_expr): Likewise.
> > (c_parser_braced_init): Likewise, for matching_braces.
> > (c_parser_paren_condition): Likewise, for matching_parens.
> > (c_parser_switch_statement): Likewise.
> > (c_parser_for_statement): Likewise.
> > (c_parser_asm_statement): Likewise.
> > (c_parser_asm_operands): Likewise.
> > (c_parser_cast_expression): Likewise.
> > (c_parser_sizeof_expression): Likewise.
> > (c_parser_alignof_expression): Likewise.
> > (c_parser_generic_selection): Likewise.
> > (c_parser_postfix_expression): Likewise for cases RID_VA_ARG,
> > RID_OFFSETOF, RID_TYPES_COMPATIBLE_P, RID_AT_SELECTOR,
> > RID_AT_PROTOCOL, RID_AT_ENCODE, reindenting as necessary.
> > In case CPP_OPEN_PAREN, pass loc_open_paren to the
> > c_parser_skip_until_found call.
> > (c_parser_objc_class_definition): Use class matching_parens as
> > above.
> > (c_parser_objc_method_decl): Likewise.
> > (c_parser_objc_try_catch_finally_statement): Likewise.
> > (c_parser_objc_synchronized_statement): Likewise.
> > (c_parser_objc_at_property_declaration): Likewise.
> > (c_parser_oacc_wait_list): Likewise.
> > (c_parser_omp_var_list_parens): Likewise.
> > (c_parser_omp_clause_collapse): Likewise.
> > (c_parser_omp_clause_default): Likewise.
> > (c_parser_omp_clause_if): Likewise.
> > (c_parser_omp_clause_num_threads): Likewise.
> > (c_parser_omp_clause_num_tasks): Likewise.
> > (c_parser_omp_clause_grainsize): Likewise.
> > (c_parser_omp_clause_priority): Likewise.
> > (c_parser_omp_clause_hint): Likewise.
> > (c_parser_omp_clause_defaultmap): Likewise.
> > (c_parser_oacc_single_int_clause): Likewise.
> > (c_parser_omp_clause_ordered): Likewise.
> > (c_parser_omp_clause_reduction): Likewise.
> > (c_parser_omp_clause_schedule): Likewise.
> > (c_parser_omp_clause_num_teams): Likewise.
> > (c_parser_omp_clause_thread_limit): Likewise.
> > (c_parser_omp_clause_aligned): Likewise.
> > (c_parser_omp_clause_linear): Likewise.
> > (c_parser_omp_clause_safelen): Likewise.
> > (c_parser_omp_clause_simdlen): Likewise.
> > (c_parser_omp_clause_depend): Likewise.
> > (c_parser_omp_clause_map): Likewise.
> > (c_parser_omp_clause_device): Likewise.
> > (c_parser_omp_clause_dist_schedule): Likewise.
> > (c_parser_omp_clause_proc_bind): Likewise.
> > (c_parser_omp_clause_uniform): Likewise.
> > (c_parser_omp_for_loop): Likewise.
> > (c_parser_cilk_clause_vectorlength): Likewise.
> > (c_parser_cilk_clause_linear): Likewise.
> > (c_parser_transaction_expression): Likewise.
> > * c-parser.h (c_parser_require): Add param matching_location
> > with
> > default UNKNOWN_LOCATION.
> > (c_parser_error): Convert return type from void to bool.
> > (c_parser_skip_until_found): Add param matching_location with
> > default UNKNOWN_LOCATION.
> > 
> > gcc/testsuite/ChangeLog:
> > * gcc.dg/unclosed-init.c: New test case.
> 
> Phew.  I only spot-checked most of the changes around the new API for
> requiring the open/close paren/brace/bracket or consuming
> parens/braces/brackets.  They were very mechanical :-)

Thanks for looking at thi

Re: [PATCH] Simplify pow with constant

[Wilco Dijkstra]

Richard Biener wrote:
> On Fri, Aug 4, 2017 at 2:26 PM, Alexander Monakov  wrote:
> > On Fri, 4 Aug 2017, Wilco Dijkstra wrote:
> >> This patch simplifies pow (C, x) into exp (x * C1), where C1 = log (C).
> >
> > I don't think you can do that for non-positive C.

True, that can be easily disallowed.

> Hmm, the question is also how this interacts with other folders like
> sqrt (pow (x, y)) -> pow (|x|, y * 0,5)?  Also we seem to miss

We fold sqrt (pow (C, x)) into pow (C, x * 0.5) first, then fold that to exp.

> pow (2, x) -> exp2 (x) and pow (10, x) -> pow10/exp10, those may
> be a better fit than exp (log (2/10) * x)?  OTOH for fast-math
> canonicalization getting rid of exp2/10 and pow10 might be beneficial.

exp10 is non-standard and doesn't have a first-class implementation in GLIBC.
Although pow (10, x) is frequently used in Fortran, I can't get exp10 emitted
by match.pd...

>> Do this only for fast-math as accuracy is reduced.  This is much faster
>> since pow is more complex than exp - with a current GLIBC the speedup
>> is more than 7 times for this transformation.
>
> Is it bound to be so on future glibc revisions and non-glibc platforms?

Yes, pow is basically log followed by exp, so exp will always be cheaper than
pow. How much will obviously vary depending on the implementation.
Szabolc's highly optimized expf has 3x throughput of the optimized powf.

> And how is accuracy affected?  I think the transform is only reasonable
> for log (C) being close to e, 2 or 10 (using exp, exp2 or exp10).  Can you
> provide an idea on whether there's a systematic error (with glibc) and
> how that behaves over the parameter space?

Accuracy depends again on the library implementation. If log (C) is accurate
(ie. far less than 0.5 ULP), and exp (x) accurate to 0.5 ULP then you get 
perfect answers over the full range.

The exp function has the largest steps close to inf - a 1 ULP change in input
changes the output by 1024 ULP (128 ULP for expf). So a 0.5ULP input error
would give ~512 ULP error if the final result is close to inf. In practice the 
output
doesn't get anywhere near inf, so ULP errors are far smaller.

> Oh, and what value of C does the benchmark that triggered this have?

10 appears quite common. I extracted a runtime log of all powf calls in SPEC
(see https://sourceware.org/ml/libc-alpha/2017-06/msg00718.html) and noticed
a lot of repetition in some inputs. Further investigation showed many uses of
pow have a constant first operand, so an obvious target for optimization.

Wilco

[PATCH] Fix OpenMP/OpenACC/Cilk+ issues with forced/non-local labels (PR c/81687)

[Jakub Jelinek]

Hi!

If there are forced or non-local labels in OpenMP parallel/task/target
regions (or OpenACC or Cilk+), then we often fail to link as the following
testcases show - the labels aren't emitted anywhere, just references to
them.

While it is true that for forced/non-local labels we can't clone the basic
blocks containing them, for OpenMP we actually aren't cloning it, but
outlining the SESE region to a new function, i.e. moving the blocks.
As such, we can move the label too.  It is UB if a goto to such a label
(computed or non-local) is performed across boundaries of the region
(from outside of the region into it or from inside of it to outside),
but it should be ok to use it within the same region, and if just printing
address or something similar we can even reference label in one function
from another one (where one of them is outlined from the other or similar).

The following patch arranges for this - does not copy such labels during
lowering, and ensures that their DECL_CONTEXT will be the function that
contains the label (rather than just any reference to it).

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2017-08-04  Jakub Jelinek  

PR c/81687
* omp-low.c (omp_copy_decl): Don't remap FORCED_LABEL or DECL_NONLOCAL
LABEL_DECLs.
* tree-cfg.c (move_stmt_op): Don't adjust DECL_CONTEXT of FORCED_LABEL
or DECL_NONLOCAL labels.
(move_stmt_r) : Adjust DECL_CONTEXT of FORCED_LABEL
or DECL_NONLOCAL labels here.

* testsuite/libgomp.c/pr81687-1.c: New test.
* testsuite/libgomp.c/pr81687-2.c: New test.

--- gcc/omp-low.c.jj2017-08-03 10:33:41.0 +0200
+++ gcc/omp-low.c   2017-08-03 15:37:52.998873566 +0200
@@ -796,6 +796,8 @@ omp_copy_decl (tree var, copy_body_data
 
   if (TREE_CODE (var) == LABEL_DECL)
 {
+  if (FORCED_LABEL (var) || DECL_NONLOCAL (var))
+   return var;
   new_var = create_artificial_label (DECL_SOURCE_LOCATION (var));
   DECL_CONTEXT (new_var) = current_function_decl;
   insert_decl_map (&ctx->cb, var, new_var);
--- gcc/tree-cfg.c.jj   2017-07-29 09:48:43.0 +0200
+++ gcc/tree-cfg.c  2017-08-03 15:37:40.395021370 +0200
@@ -6718,7 +6718,15 @@ move_stmt_op (tree *tp, int *walk_subtre
*tp = t = out->to;
}
 
- DECL_CONTEXT (t) = p->to_context;
+ /* For FORCED_LABELs we can end up with references from other
+functions if some SESE regions are outlined.  It is UB to
+jump in between them, but they could be used just for printing
+addresses etc.  In that case, DECL_CONTEXT on the label should
+be the function containing the glabel stmt with that LABEL_DECL,
+rather than whatever function a reference to the label was seen
+last time.  */
+ if (!FORCED_LABEL (t) && !DECL_NONLOCAL (t))
+   DECL_CONTEXT (t) = p->to_context;
}
   else if (p->remap_decls_p)
{
@@ -6836,6 +6844,21 @@ move_stmt_r (gimple_stmt_iterator *gsi_p
 case GIMPLE_OMP_RETURN:
 case GIMPLE_OMP_CONTINUE:
   break;
+
+case GIMPLE_LABEL:
+  {
+   /* For FORCED_LABEL, move_stmt_op doesn't adjust DECL_CONTEXT,
+  so that such labels can be referenced from other regions.
+  Make sure to update it when seeing a GIMPLE_LABEL though,
+  that is the owner of the label.  */
+   walk_gimple_op (stmt, move_stmt_op, wi);
+   *handled_ops_p = true;
+   tree label = gimple_label_label (as_a  (stmt));
+   if (FORCED_LABEL (label) || DECL_NONLOCAL (label))
+ DECL_CONTEXT (label) = p->to_context;
+  }
+  break;
+
 default:
   if (is_gimple_omp (stmt))
{
--- libgomp/testsuite/libgomp.c/pr81687-1.c.jj  2017-08-03 15:43:32.832888380 
+0200
+++ libgomp/testsuite/libgomp.c/pr81687-1.c 2017-08-03 15:43:06.0 
+0200
@@ -0,0 +1,23 @@
+/* PR c/81687 */
+/* { dg-do link } */
+/* { dg-additional-options "-O2" } */
+
+extern int printf (const char *, ...);
+
+int
+main ()
+{
+  #pragma omp parallel
+  {
+   lab1:
+printf ("lab1=%p\n", (void *)(&&lab1));
+  }
+ lab2:
+  #pragma omp parallel
+  {
+   lab3:
+printf ("lab2=%p\n", (void *)(&&lab2));
+  }
+  printf ("lab3=%p\n", (void *)(&&lab3));
+  return 0;
+}
--- libgomp/testsuite/libgomp.c/pr81687-2.c.jj  2017-08-03 15:43:36.229848545 
+0200
+++ libgomp/testsuite/libgomp.c/pr81687-2.c 2017-08-03 15:43:15.0 
+0200
@@ -0,0 +1,27 @@
+/* PR c/81687 */
+/* { dg-do link } */
+/* { dg-additional-options "-O2" } */
+
+int
+main ()
+{
+  __label__ lab4, lab5, lab6;
+  volatile int l = 0;
+  int m = l;
+  void foo (int x) { if (x == 1) goto lab4; }
+  void bar (int x) { if (x == 2) goto lab5; }
+  void baz (int x) { if (x == 3) goto lab6; }
+  #pragma omp parallel
+  {
+foo (m + 1);
+   lab4:;
+  }
+  #pragma omp task
+  {
+bar (m + 2);
+   lab5:;
+  }
+  baz (m + 3);
+ lab6:;
+  return 0;
+}

  

Re: [PATCH] i386: Rewrite check for AVX512 features

[Uros Bizjak]

On Fri, Aug 4, 2017 at 12:19 PM, Peryt, Sebastian
 wrote:
>> -Original Message-
>> From: Uros Bizjak [mailto:ubiz...@gmail.com]
>> Sent: Sunday, July 30, 2017 11:02 AM
>> To: H.J. Lu 
>> Cc: gcc-patches@gcc.gnu.org; Koval, Julia ; Peryt,
>> Sebastian 
>> Subject: Re: [PATCH] i386: Rewrite check for AVX512 features
>>
>> On Sat, Jul 29, 2017 at 3:06 PM, H.J. Lu  wrote:
>> > Add a new file, avx512-check.h, to check all AVX512 features.  The
>> > test is skipped if any requested AVX512 features are unavailable.
>> >
>> > Tested on Skylake server and Haswell.  OK for trunk?
>>
>> No, I'd rather leave it in in the way they are now, so test can include 
>> individual
>> checks.
>>
>> Uros.
>>
> Uros,
>
> Can you please suggests any alternative approach? The main problem with 
> current
> one used in avx512f-helper.h is that it doesn't take into account situations 
> where
> two features are required, but only one is supported by CPU.
>
> That's exactly the case with AVX512VL and AVX512VBMI on SKX. Once 
> avx512vl-check.h
> verifies existence of AVX512VL on SKX it starts to execute test, which fails 
> because
> AVX512VBMI is not supported but it has never been checked, before test 
> execution.
>
> Honestly I cannot think of any solution that would allow for both individual 
> include
> files (beside what HJ already did in those few remaining tests) and multiple 
> features
> verification. Also I think it's worth taking into account that not many tests 
> actually
> use individual include files instead of avx512f-helper.h.

In the past, required ISA combinations were handled by introducing
"combined" header files (i.e. pclmul-avx-check.h).

However, with AVX512F, we already have different approach (I didn't
notice, TBH) with avx512f-helper.h and lots of #ifdeffery inside. I
didn' t like this ifdeffery, but comparing existing and new one, the
new one is definitely an improvement.

So, let's move forward with the patch.

OK for mainline.

Thanks,
Uros.

>
> Thanks,
> Sebastian
>> >
>> > H.J.
>> > ---
>> > PR target/81590
>> > * gcc.target/i386/avx512-check.h: New file.
>> > * gcc.target/i386/avx5124fmaps-check.h: Removed.
>> > * gcc.target/i386/avx5124vnniw-check.h: Likewise.
>> > * gcc.target/i386/avx512cd-check.h: Likewise.
>> > * gcc.target/i386/avx512ifma-check.h: Likewise.
>> > * gcc.target/i386/avx512vbmi-check.h: Likewise.
>> > * gcc.target/i386/avx512vpopcntdq-check.h: Likewise.
>> > * gcc.target/i386/avx512bw-check.h: Rewrite.
>> > * gcc.target/i386/avx512dq-check.h: Likewise.
>> > * gcc.target/i386/avx512er-check.h: Likewise.
>> > * gcc.target/i386/avx512f-check.h: Likewise.
>> > * gcc.target/i386/avx512vl-check.h: Likewise.
>> > * gcc.target/i386/avx512f-helper.h: Include "avx512-check.h"
>> > only.
>> > (test_512): Removed.
>> > (avx512*_test): Likewise.
>> > * gcc.target/i386/avx512f-pr71559.c (TEST): Undef.
>> > ---
>> >  gcc/testsuite/gcc.target/i386/avx512-check.h   | 113
>> +
>> >  gcc/testsuite/gcc.target/i386/avx5124fmaps-check.h |  47 -
>> > gcc/testsuite/gcc.target/i386/avx5124vnniw-check.h |  47 -
>> >  gcc/testsuite/gcc.target/i386/avx512bw-check.h |  50 +
>> >  gcc/testsuite/gcc.target/i386/avx512cd-check.h |  46 -
>> >  gcc/testsuite/gcc.target/i386/avx512dq-check.h |  50 +
>> >  gcc/testsuite/gcc.target/i386/avx512er-check.h |  49 +
>> >  gcc/testsuite/gcc.target/i386/avx512f-check.h  |  49 +
>> >  gcc/testsuite/gcc.target/i386/avx512f-helper.h |  64 +---
>> >  gcc/testsuite/gcc.target/i386/avx512f-pr71559.c|   1 +
>> >  gcc/testsuite/gcc.target/i386/avx512ifma-check.h   |  46 -
>> >  gcc/testsuite/gcc.target/i386/avx512vbmi-check.h   |  46 -
>> >  gcc/testsuite/gcc.target/i386/avx512vl-check.h |  51 +-
>> >  .../gcc.target/i386/avx512vpopcntdq-check.h|  47 -
>> >  14 files changed, 130 insertions(+), 576 deletions(-)  create mode
>> > 100644 gcc/testsuite/gcc.target/i386/avx512-check.h
>> >  delete mode 100644 gcc/testsuite/gcc.target/i386/avx5124fmaps-check.h
>> >  delete mode 100644 gcc/testsuite/gcc.target/i386/avx5124vnniw-check.h
>> >  delete mode 100644 gcc/testsuite/gcc.target/i386/avx512cd-check.h
>> >  delete mode 100644 gcc/testsuite/gcc.target/i386/avx512ifma-check.h
>> >  delete mode 100644 gcc/testsuite/gcc.target/i386/avx512vbmi-check.h
>> >  delete mode 100644
>> > gcc/testsuite/gcc.target/i386/avx512vpopcntdq-check.h
>> >
>> > diff --git a/gcc/testsuite/gcc.target/i386/avx512-check.h
>> > b/gcc/testsuite/gcc.target/i386/avx512-check.h
>> > new file mode 100644
>> > index 000..bfe14960100
>> > --- /dev/null
>> > +++ b/gcc/testsuite/gcc.target/i386/avx512-check.h
>> > @@ -0,0 +1,113 @@
>> > +#include 
>> > +#include "cpuid.h"
>> > +#include "m512-check.h"
>> 

[PATCH] Fix switch expansion (PR middle-end/81698)

[Jakub Jelinek]

Hi!

As mentioned inthe PR, the switch expansion relies on EDGE_SUCC (bb, 0)
from a GIMPLE_SWITCH to be the default edge, but GIMPLE only ensures that
default label is the first one.

So, this patch (in expand_case) instead finds the default_edge from the
default label and corresponding block.
Another issue is emit_case_dispatch_table - this function is called from
expand_case as well as sjlj landing pad handling.  For the former, it
suffers from a similar problem, and additionally we could have removed
the default edge already, so even for switches that formerly had a default
label we suddenly consider a random different edge as the default.
For sjlj, I've just looked at one testcase in cross to hppa2.0w-hp-hpux10.0,
and I believe stmt_bb should be just NULL, in any case if it would be
non-NULL and there is some edge, it would hardly be the default edge,
since the caller is emitting the default label after the switch expansion.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2017-08-04  Jakub Jelinek  

PR middle-end/81698
* stmt.c (emit_case_dispatch_table): Add DEFAULT_EDGE argument,
instead of computing it in the function.  Formatting fix.
(expand_case): Don't rely on default_edge being the first edge,
clear it if removing it, pass default_edge to
emit_case_dispatch_table.
(expand_sjlj_dispatch_table): Pass NULL as DEFAULT_EDGE, formatting
fix.

--- gcc/stmt.c.jj   2017-07-14 13:04:47.0 +0200
+++ gcc/stmt.c  2017-08-04 14:36:28.936447265 +0200
@@ -945,8 +945,8 @@ conditional_probability (profile_probabi
 static void
 emit_case_dispatch_table (tree index_expr, tree index_type,
  struct case_node *case_list, rtx default_label,
- tree minval, tree maxval, tree range,
-  basic_block stmt_bb)
+ edge default_edge,  tree minval, tree maxval,
+ tree range, basic_block stmt_bb)
 {
   int i, ncases;
   struct case_node *n;
@@ -954,7 +954,6 @@ emit_case_dispatch_table (tree index_exp
   rtx_insn *fallback_label = label_rtx (case_list->code_label);
   rtx_code_label *table_label = gen_label_rtx ();
   bool has_gaps = false;
-  edge default_edge = stmt_bb ? EDGE_SUCC (stmt_bb, 0) : NULL;
   profile_probability default_prob = default_edge ? default_edge->probability
  : profile_probability::never 
();
   profile_probability base = get_outgoing_edge_probs (stmt_bb);
@@ -1026,9 +1025,8 @@ emit_case_dispatch_table (tree index_exp
  through the indirect jump or the direct conditional jump
  before that. Split the probability of reaching the
  default label among these two jumps.  */
-  new_default_prob = conditional_probability (default_prob.apply_scale
-(1, 2),
-  base);
+  new_default_prob
+   = conditional_probability (default_prob.apply_scale (1, 2), base);
   default_prob = default_prob.apply_scale (1, 2);
   base -= default_prob;
 }
@@ -1147,9 +1145,10 @@ expand_case (gswitch *stmt)
   do_pending_stack_adjust ();
 
   /* Find the default case target label.  */
-  default_label = jump_target_rtx
-  (CASE_LABEL (gimple_switch_default_label (stmt)));
-  edge default_edge = EDGE_SUCC (bb, 0);
+  tree default_lab = CASE_LABEL (gimple_switch_default_label (stmt));
+  default_label = jump_target_rtx (default_lab);
+  basic_block default_bb = label_to_block_fn (cfun, default_lab);
+  edge default_edge = find_edge (bb, default_bb);
   profile_probability default_prob = default_edge->probability;
 
   /* Get upper and lower bounds of case values.  */
@@ -1245,9 +1244,10 @@ expand_case (gswitch *stmt)
{
  default_label = NULL;
  remove_edge (default_edge);
+ default_edge = NULL;
}
   emit_case_dispatch_table (index_expr, index_type,
-   case_list, default_label,
+   case_list, default_label, default_edge,
minval, maxval, range, bb);
 }
 
@@ -1340,9 +1340,9 @@ expand_sjlj_dispatch_table (rtx dispatch
}
 
   emit_case_dispatch_table (index_expr, index_type,
-   case_list, default_label,
+   case_list, default_label, NULL,
minval, maxval, range,
-BLOCK_FOR_INSN (before_case));
+   BLOCK_FOR_INSN (before_case));
   emit_label (default_label);
 }
 

Jakub

[PATCH][AArch64] PR60580: Fix frame pointer option magic

[Wilco Dijkstra]

To fix PR60580 simplify the logic in aarch64_override_options_after_change_1 
(). 
If the frame pointer is enabled, set it to a special value that behaves similar
to frame pointer omission.  If we don't do this all leaf functions will get a
frame pointer even if flag_omit_leaf_frame_pointer is set.

If flag_omit_frame_pointer has this special value, we must force the frame
pointer if not in a leaf function.  We also need to force it in a leaf function
if flag_omit_frame_pointer is not set or if LR is used.

Doing this allows both -fomit-frame-pointer and -fomit-leaf-frame-pointer to be
independently set and changed in each function with the expected behaviour.

OK for commit and backport to GCC7/GCC6?

ChangeLog:
2017-08-04  Wilco Dijkstra  

gcc/
PR middle-end/60580
* config/aarch64/aarch64.c (aarch64_frame_pointer_required)
Check special value of flag_omit_frame_pointer.
(aarch64_can_eliminate): Likewise.
(aarch64_override_options_after_change_1): Simplify handling of
-fomit-frame-pointer and -fomit-leaf-frame-pointer.

--
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 
f90420bffe841155d2ea12dc7450fa699718e8ae..8ec196bca5e7203f2bb38381fe89b1aec43b9a29
 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -2839,12 +2839,13 @@ aarch64_output_probe_stack_range (rtx reg1, rtx reg2)
 static bool
 aarch64_frame_pointer_required (void)
 {
-  /* In aarch64_override_options_after_change
- flag_omit_leaf_frame_pointer turns off the frame pointer by
- default.  Turn it back on now if we've not got a leaf
- function.  */
-  if (flag_omit_leaf_frame_pointer
-  && (!crtl->is_leaf || df_regs_ever_live_p (LR_REGNUM)))
+  /* Use the frame pointer if enabled and it is not a leaf function, unless
+ leaf frame pointer omission is disabled.  If the frame pointer is enabled,
+ force the frame pointer in leaf functions which use LR.  */
+  if (flag_omit_frame_pointer == 2
+  && !(flag_omit_leaf_frame_pointer
+  && crtl->is_leaf
+  && !df_regs_ever_live_p (LR_REGNUM)))
 return true;
 
   /* Force a frame pointer for EH returns so the return address is at FP+8.  */
@@ -5892,6 +5893,7 @@ aarch64_can_eliminate (const int from, const int to)
 LR in the function, then we'll want a frame pointer after all, so
 prevent this elimination to ensure a frame pointer is used.  */
   if (to == STACK_POINTER_REGNUM
+ && flag_omit_frame_pointer == 2
  && flag_omit_leaf_frame_pointer
  && df_regs_ever_live_p (LR_REGNUM))
return false;
@@ -8914,24 +8916,16 @@ aarch64_parse_override_string (const char* input_string,
 static void
 aarch64_override_options_after_change_1 (struct gcc_options *opts)
 {
-  /* The logic here is that if we are disabling all frame pointer generation
- then we do not need to disable leaf frame pointer generation as a
- separate operation.  But if we are *only* disabling leaf frame pointer
- generation then we set flag_omit_frame_pointer to true, but in
- aarch64_frame_pointer_required we return false only for leaf functions.
-
- PR 70044: We have to be careful about being called multiple times for the
- same function.  Once we have decided to set flag_omit_frame_pointer just
- so that we can omit leaf frame pointers, we must then not interpret a
- second call as meaning that all frame pointer generation should be
- omitted.  We do this by setting flag_omit_frame_pointer to a special,
- non-zero value.  */
-  if (opts->x_flag_omit_frame_pointer == 2)
-opts->x_flag_omit_frame_pointer = 0;
-
-  if (opts->x_flag_omit_frame_pointer)
-opts->x_flag_omit_leaf_frame_pointer = false;
-  else if (opts->x_flag_omit_leaf_frame_pointer)
+  /* PR 70044: We have to be careful about being called multiple times for the
+ same function.  This means all changes should be repeatable.  */
+
+  /* If the frame pointer is enabled, set it to a special value that behaves
+ similar to frame pointer omission.  If we don't do this all leaf functions
+ will get a frame pointer even if flag_omit_leaf_frame_pointer is set.
+ If flag_omit_frame_pointer has this special value, we must force the
+ frame pointer if not in a leaf function.  We also need to force it in a
+ leaf function if flag_omit_frame_pointer is not set or if LR is used.  */
+  if (opts->x_flag_omit_frame_pointer == 0)
 opts->x_flag_omit_frame_pointer = 2;
 
   /* If not optimizing for size, set the default

Re: [PING^4][PATCH v2] Generate reproducible output independently of the build-path

[Yury Gribov]

On Fri, Aug 4, 2017 at 1:32 PM, Matthias Klose  wrote:
> On 03.08.2017 12:05, Jeff Law wrote:
>> On 08/02/2017 08:06 PM, Ximin Luo wrote:
>>> Jeff Law:
 On 07/21/2017 10:15 AM, Ximin Luo wrote:
> (Please keep me on CC, I am not subscribed)
>
>
> Proposal
> 
>
> This patch series adds a new environment variable BUILD_PATH_PREFIX_MAP. 
> When
> this is set, GCC will treat this as extra implicit 
> "-fdebug-prefix-map=$value"
> command-line arguments that precede any explicit ones. This makes the 
> final
> binary output reproducible, and also hides the unreproducible value (the 
> source
> path prefixes) from CFLAGS et. al. which many build tools (understandably)
> embed as-is into their build output.
 I'd *really* avoid doing this with magic environment variables.  Make it
 a first class option to the compiler.  Yes, it means projects that want
 this behavior have to arrange to pass that flag to their compiler, but
 IMHO that's much preferred over environment variables.

 Jeff

>>>
>>> Hi Jeff,
>>>
>>> If by "first class option" you meant a command-line flag, GCC *already has* 
>>> that (-fdebug-prefix-map) and it wasn't enough to achieve reproducibility 
>>> in many cases we tested. dpkg-buildflags actually already adds these flags 
>>> to CFLAGS CXXFLAGS etc on Debian. However, with this patch using the 
>>> environment variable, we are able to reproduce 1800 more packages out of 
>>> 26000.Then take what you've done with the environment variable and instead
>> implement it on top of a switch.  An environment variable is absolutely
>> the wrong thing to do here.
>>
>>>
>>> GCC already supports a similar environment variable SOURCE_DATE_EPOCH, 
>>> which was accepted about 2 years ago in a patch written by one of our GSoC 
>>> students. We are not planning any more environment variables like this, and 
>>> are committed to fixing other sources of non-determinism by patching the 
>>> relevant build scripts.
>> I would have rejected that as well :-)  One of the few times I would
>> have disagreed with Bernd.
>
> You can argue that gcc has command line options to set these, but most build
> systems can be influenced by well documented environment variables like 
> CFLAGS,
> CXXFLAGS, LDFLAGS, so adding one more variable like SOURCE_DATE_EPOCH makes
> sense, considering that many tools dealing with timestamps don't even have
> command line options to do these (and there it's not just about compilers).
>
> BUILD_PATH_PREFIX_MAP might be passed as well in CFLAGS / CXXFLAGS, but there
> are build systems that even ignore these environment variables, and it's 
> common
> to require the use of environment variables for distributions, like
>
> http://pkgs.fedoraproject.org/cgit/rpms/gcc.git/tree/gcc7-foffload-default.patch
>
> If changes like that are not allowed to go upstream, where do you want to keep
> these instead, and keeping these in sync across distributions?

In general any distro provides some way to set default parameters for
every package.  Adding hacks to compiler to work around limitations of
particular build system does not sound right.

Here's a relevant discussion from binutils ML:
https://sourceware.org/ml/binutils/2009-05/msg00086.html

-Y

Re: [C++ committed] Fix ICE with -Wshadow=compatible-local (PR c++/81640)

[Jason Merrill]

On Wed, Aug 2, 2017 at 3:38 AM, Jakub Jelinek  wrote:
> I've bootstrapped/regtested the following patch last night and committed
> after preapproval from Nathan, though of course a question is if
> lookup_fnfields_slot/lookup_fnfields_slot_nolazy shouldn't have
> if (!CLASS_TYPE_P (type)) return NULL_TREE; or
> gcc_assert (!CLASS_TYPE_P (type));
> readded to the beginning of those functions.

It seems that we already ICE if we call it with a non-class type, so
adding an assert would just document that behavior?

Jason

Re: [PATCH v2][RFC] Canonize names of attributes.

[Jason Merrill]

On Fri, Aug 4, 2017 at 9:42 AM, Martin Liška  wrote:
> On 08/02/2017 01:25 PM, Joseph Myers wrote:
>> On Thu, 13 Jul 2017, Martin Liška wrote:
>>
>>> +/* For a given IDENTIFIER_NODE, strip leading and trailing '_' characters
>>> +   so that we have a canonical form of attribute names.  */
>>> +
>>> +static inline tree
>>> +canonicalize_attr_name (tree attr_name)
>>> +{
>>> +  const size_t l = IDENTIFIER_LENGTH (attr_name);
>>> +  const char *s = IDENTIFIER_POINTER (attr_name);
>>> +
>>> +  if (l > 4 && s[0] == '_')
>>> +{
>>> +  gcc_checking_assert (s[l - 2] == '_');
>>> +  return get_identifier_with_length (s + 2, l - 4);
>>> +}
>>> +
>>> +  return attr_name;
>>
>> For this to (a) be correct, (b) not trigger the assertion, there must be a
>> precondition that attr_name either starts and ends with __, or does not
>> start with _, or has length 4 or less.  I don't see anything in the
>> callers to ensure this precondition holds, so that, for example,
>> __attribute__ ((_foobar)) does not trigger the assertion, and
>> __attribute__ ((_xformat__)) is not wrongly interpreted as a format
>> attribute (and similarly for attribute arguments when those are
>> canonicalized).
>
> Thanks for the review. I've updated to to canonicalize just __.+__.
> I added test-case for that: gcc/testsuite/gcc.dg/Wattributes-5.c.
>
>>
>>> +/* Compare attribute identifiers ATTR1 and ATTR2 with length ATTR1_LEN and
>>> +   ATTR2_LEN.  */
>>> +
>>> +static inline bool
>>> +cmp_attribs (const char *attr1, size_t attr1_len,
>>> + const char *attr2, size_t attr2_len)
>>> +{
>>> +  gcc_checking_assert (attr1_len == 0 || attr1[0] != '_');
>>> +  gcc_checking_assert (attr2_len == 0 || attr2[0] != '_');
>
> And I removed these asserts.
>
> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
>
> Ready to be installed?

OK.

Jason

Re: [C++ committed] Fix ICE with -Wshadow=compatible-local (PR c++/81640)

[Jakub Jelinek]

On Fri, Aug 04, 2017 at 12:50:36PM -0400, Jason Merrill wrote:
> On Wed, Aug 2, 2017 at 3:38 AM, Jakub Jelinek  wrote:
> > I've bootstrapped/regtested the following patch last night and committed
> > after preapproval from Nathan, though of course a question is if
> > lookup_fnfields_slot/lookup_fnfields_slot_nolazy shouldn't have
> > if (!CLASS_TYPE_P (type)) return NULL_TREE; or
> > gcc_assert (!CLASS_TYPE_P (type));
> > readded to the beginning of those functions.
> 
> It seems that we already ICE if we call it with a non-class type, so
> adding an assert would just document that behavior?

CLASSTYPE_METHOD_VEC doesn't exactly check CLASS_TYPE_P if checking,
it just requires that LANG_TYPE_CLASS_CHECK is non-NULL (dunno if there
are any non-CLASS_TYPE_P types that have it non-NULL, or any CLASS_TYPE_P
types that have it NULL).
But yes, for checking builds it would be primarily documentation, for
checking builds likely too, because if TYPE_LANG_SPECIFIC is NULL,
then CLASSTYPE_METHOD_VEC will dereference NULL.

Jakub

Re: [C++ Patch] PR 79790 ("[C++17] ICE class template argument deduction")

[Jason Merrill]

On 07/14/2017 01:32 PM, Paolo Carlini wrote:

+  error ("cannot deduce template arguments for copy-initialization"
+" of %qT, as it has no viable implicit deduction guides", type);


Why "copy-initialization"?  We do deduction for direct-initialization, 
as well.


I would also drop the "implicit".

OK with those tweaks.

Jason

[PATCH, rs6000] Move vec_mulo() and vec_mule() builtin test cases to P8 test file.

[Carl Love]

GCC Maintainers:

The following patch moves the vec_mule() and vec_mulo() builtin test
cases to the builtins-3-p8.c test case file.  These builtins use P8
instructions so they need to be in the P8 test file.

Please let me know if the following patch is acceptable.  Thanks.

Carl Love

--

gcc/testsuite/ChangeLog:

2017-08-04  Carl Love  

* gcc.target/powerpc/builtins-3.c: Remove builtin test cases for
vec_mule, and vec_mulo.
* gcc.target/powerpc/builtins-3-p8.c: Add builtin test cases for
vec_mule, and vec_mulo.
---
 gcc/testsuite/gcc.target/powerpc/builtins-3-p8.c | 35 +++-
 gcc/testsuite/gcc.target/powerpc/builtins-3.c| 32 --
 2 files changed, 34 insertions(+), 33 deletions(-)

diff --git a/gcc/testsuite/gcc.target/powerpc/builtins-3-p8.c 
b/gcc/testsuite/gcc.target/powerpc/builtins-3-p8.c
index 3baa1d8..bc1c850 100644
--- a/gcc/testsuite/gcc.target/powerpc/builtins-3-p8.c
+++ b/gcc/testsuite/gcc.target/powerpc/builtins-3-p8.c
@@ -85,6 +85,30 @@ test_vss_mradds_vss_vss (vector signed short x, vector 
signed short y,
return vec_mradds (x, y, z);
 }
 
+vector signed long long
+test_vsll_mule_vsi_vsi (vector signed int x, vector signed int y)
+{
+   return vec_mule (x, y);
+}
+
+vector unsigned long long
+test_vull_mule_vui_vui (vector unsigned int x, vector unsigned int y)
+{
+   return vec_mule (x, y);
+}
+
+vector signed long long
+test_vsll_mulo_vsi_vsi (vector signed int x, vector signed int y)
+{
+   return vec_mulo (x, y);
+}
+
+vector unsigned long long
+test_vull_mulo_vui_vui (vector unsigned int x, vector unsigned int y)
+{
+   return vec_mulo (x, y);
+}
+
 /* Expected test results:
 
  test_eq_long_long 1 vcmpequd inst
@@ -98,7 +122,12 @@ test_vss_mradds_vss_vss (vector signed short x, vector 
signed short y,
  test_unsigned_int_popcnt_signed_int   2 vpopcntw
  test_unsigned_int_popcnt_unsigned_int 1 vpopcntd
  test_unsigned_long_long_popcnt_unsigned_long 1 vpopcntd
- test_vss_mradds_vss_vsss  1 vmhraddshs */
+ test_vss_mradds_vss_vsss  1 vmhraddshs
+ test_vsll_mulo_vsi_vsi1 vmulosw
+ test_vull_mulo_vui_vui1 vmulouw
+ test_vsll_mule_vsi_vsi1 vmulesw
+ test_vull_mule_vui_vui1 vmuleuw
+ */
 
 /* { dg-final { scan-assembler-times "vcmpequd" 1 } } */
 /* { dg-final { scan-assembler-times "vpkudum"  1 } } */
@@ -109,3 +138,7 @@ test_vss_mradds_vss_vss (vector signed short x, vector 
signed short y,
 /* { dg-final { scan-assembler-times "vpopcntw" 2 } } */
 /* { dg-final { scan-assembler-times "vpopcntd" 2 } } */
 /* { dg-final { scan-assembler-times "vmhraddshs"  1 } } */
+/* { dg-final { scan-assembler-times "vmulosw"  1 } } */
+/* { dg-final { scan-assembler-times "vmulouw"  1 } } */
+/* { dg-final { scan-assembler-times "vmulesw"  1 } } */
+/* { dg-final { scan-assembler-times "vmuleuw"  1 } } */
diff --git a/gcc/testsuite/gcc.target/powerpc/builtins-3.c 
b/gcc/testsuite/gcc.target/powerpc/builtins-3.c
index 00fa6ec..42153da 100644
--- a/gcc/testsuite/gcc.target/powerpc/builtins-3.c
+++ b/gcc/testsuite/gcc.target/powerpc/builtins-3.c
@@ -112,30 +112,6 @@ test_vull_slo_vull_vuc (vector unsigned long long x, 
vector unsigned char y)
return vec_slo (x, y);
 }
 
-vector signed long long
-test_vsll_mule_vsi_vsi (vector signed int x, vector signed int y)
-{
-   return vec_mule (x, y);
-}
-
-vector unsigned long long
-test_vull_mule_vui_vui (vector unsigned int x, vector unsigned int y)
-{
-   return vec_mule (x, y);
-}
-
-vector signed long long
-test_vsll_mulo_vsi_vsi (vector signed int x, vector signed int y)
-{
-   return vec_mulo (x, y);
-}
-
-vector unsigned long long
-test_vull_mulo_vui_vui (vector unsigned int x, vector unsigned int y)
-{
-   return vec_mulo (x, y);
-}
-
 vector signed char
 test_vsc_sldw_vsc_vsc (vector signed char x, vector signed char y)
 {
@@ -207,10 +183,6 @@ test_vul_sldw_vul_vul (vector unsigned long long x,
  test_vsll_slo_vsll_vuc1 vslo
  test_vull_slo_vsll_vsc1 vslo
  test_vull_slo_vsll_vuc1 vslo
- test_vsll_mulo_vsi_vsi1 vmulosw
- test_vull_mulo_vui_vui1 vmulouw
- test_vsll_mule_vsi_vsi1 vmulesw
- test_vull_mule_vui_vui1 vmuleuw
  test_vsc_mulo_vsc_vsc 1 xxsldwi
  test_vuc_mulo_vuc_vuc 1 xxsldwi
  test_vssi_mulo_vssi_vssi  1 xxsldwi
@@ -236,8 +208,4 @@ test_vul_sldw_vul_vul (vector unsigned long long x,
 /* { dg-final { scan-assembler-times "xvnegsp"  1 } } */
 /* { dg-final { scan-assembler-times "xvnegdp"  1 } } */
 /* { dg-final { scan-assembler-times "vslo" 4 } } */
-/* { dg-final { scan-assembler-times "vmulosw"  1 } } */
-/* { dg-final { scan-assembler-times "vmulouw"  1 } } */
-/* { dg-

Re: [C++ Patch] PR 79790 ("[C++17] ICE class template argument deduction")

[Paolo Carlini]

Hi,

On 04/08/2017 19:00, Jason Merrill wrote:

On 07/14/2017 01:32 PM, Paolo Carlini wrote:

+  error ("cannot deduce template arguments for copy-initialization"
+ " of %qT, as it has no viable implicit deduction guides", 
type);


Why "copy-initialization"?  We do deduction for direct-initialization, 
as well.


I would also drop the "implicit".

OK with those tweaks.

Thanks Jason for your review and, essentially, approval of the work.

About the exact wording, I'm a little puzzled, because, besides the 
"implicit" nit, the "copy-inizialization" already occurs in two other 
places in that function, in the preceding:


  if (elided && !cands)
{
  error ("cannot deduce template arguments for copy-initialization"
 " of %qT, as it has no non-explicit deduction guides or "
 "user-declared constructors", type);
  return error_mark_node;
}

and in the following:

  if (elided)
inform (input_location, "explicit deduction guides not considered "
"for copy-initialization");

and now I'm wondering which, if any, should be also removed?!? Both? Can 
you help me about that?


Thanks,
Paolo.

Re: [PATCH] Fix PR81354 (rewrite gimple_split_edge)

[Bill Schmidt]

On Aug 1, 2017, at 10:56 AM, Bill Schmidt  wrote:
> 
> On Aug 1, 2017, at 8:50 AM, Bill Schmidt  wrote:
>> 
>> On Aug 1, 2017, at 7:44 AM, Bill Schmidt  wrote:
>>> 
 
 On Aug 1, 2017, at 3:46 AM, Richard Biener  
 wrote:
 
 On Mon, Jul 31, 2017 at 4:03 PM, Bill Schmidt
  wrote:
> 
>> On Jul 31, 2017, at 8:19 AM, Bill Schmidt  
>> wrote:
>> 
>> That would certainly be much simpler!  I'll regstrap it and test it on 
>> the other
>> occurrence I've found to be certain.
> 
> Unfortunately, this fails bootstrap:
> 
> /home/wschmidt/gcc/gcc-mainline-test/gcc/calls.c: In function 'rtx_def* 
> emit_library_call_value_1(int, rtx, rtx, libcall_type, machine_mode, int, 
> va_list)':
> /home/wschmidt/gcc/gcc-mainline-test/gcc/calls.c:4362:1: error: 
> definition in block 214 does not dominate use in block 14
> emit_library_call_value_1 (int retval, rtx orgfun, rtx value,
> ^
> for SSA_NAME: slsr_772 in statement:
> slsr_749 = PHI <_17(26), slsr_772(14), slsr_334(214)>
> PHI argument
> slsr_772
> for PHI node
> slsr_749 = PHI <_17(26), slsr_772(14), slsr_334(214)>
> during GIMPLE pass: slsr
> /home/wschmidt/gcc/gcc-mainline-test/gcc/calls.c:4362:1: internal 
> compiler error: verify_ssa failed
> 0x11567cf3 verify_ssa(bool, bool)
> /home/wschmidt/gcc/gcc-mainline-test/gcc/tree-ssa.c:1186
> 0x10fc3fff execute_function_todo
> /home/wschmidt/gcc/gcc-mainline-test/gcc/passes.c:1997
> 0x10fc277f do_per_function
> /home/wschmidt/gcc/gcc-mainline-test/gcc/passes.c:1655
> 0x10fc42a3 execute_todo
> /home/wschmidt/gcc/gcc-mainline-test/gcc/passes.c:2044
> Please submit a full bug report,
> with preprocessed source if appropriate.
> Please include the complete backtrace with any bug report.
> See  for instructions.
> 
> Not terribly surprising given how sensitive this stuff is.  I can look 
> into why
> this fails, but looks like it can't be quite this simple, sadly.
 
 Intersting ... a dg-torture.exp run was clean for me (all I
 tested...).  So yes, can you
 check what fails?  Maybe run the testsuite with the stage1 compiler.
>>> 
>>> Looks like it's the stage1 that doesn't build.  I think the difference is
>>> that I was building trunk and you were building 6.  I'll try to look into
>>> it later today after I get through some meetings.
>> 
>> Sorry, no, it was stage 2 where the failure occurs.
> 
> Ah, "good" news.  I believe the bootstrap failure with this change is another
> rediscovery of https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81503, which
> explains why it wasn't seen for GCC 6.  I'll work on getting that fixed and
> then try this again.

Well, no.  I added the fix for 81503 to your patch, and I still get a failure on
trunk.  For the time being I am going to look at the edge-insertion idea 
before spending any more time on gimple_split_edge.

Bill

> 
> Bill
> 
>> 
>> Bill
>> 
>>> 
>>> Bill
 
 Richard.
 
> Bill
> 
>> 
>> -- Bill
>> 
>>> On Jul 31, 2017, at 4:15 AM, Richard Biener 
>>>  wrote:
>>> 
>>> On Sun, Jul 30, 2017 at 8:04 PM, Bill Schmidt
>>>  wrote:
 Hi,
 
 PR81354 identifies a latent bug that can happen in SLSR since the
 conditional candidate support was first added.  SLSR relies on the
 address of a GIMPLE PHI remaining constant during the course of the
 optimization pass, but it needs to split edges.  The use of
 make_single_succ_edge and reinstall_phi_args in gimple_split_edge
 causes GIMPLE PHI statements to be temporarily expanded to add a
 predecessor, and then rebuilt to have the original number of
 predecessors.  The expansion usually, if not always, causes the PHI
 statement to change address.  Thus gimple_split_edge needs to be
 rewritten to perform in-situ replacement of PHI arguments.
 
 The required pieces of make_single_succ_edge have been extracted into
 two places:  make_replacement_pred_edge, and some fixup code at the
 end of gimple_split_edge.  The division is necessary because the
 destination of the original edge must remember its original
 predecessors for the switch processing in
 gimple_redirect_edge_and_branch_1 to work properly.
 
 The function gimple_redirect_edge_and_branch was factored into two
 pieces so that most of it can be used by gimple_split_edge without
 calling ssa_redirect_edge, which also interferes with PHIs.  The
 useful bits of ssa_redirect_edge are factored out into the next three
 lines of gimple_split_edge.
 
 Similarly, redirect_eh_edge had already been similarly factored into
 redirect_eh_edge_1 and ssa_redirect_edge.  I

Re: [C++ Patch] PR 79790 ("[C++17] ICE class template argument deduction")

[Tim Song]

On Fri, Aug 4, 2017 at 1:32 PM, Paolo Carlini  wrote:

> About the exact wording, I'm a little puzzled, because, besides the
> "implicit" nit, the "copy-inizialization" already occurs in two other places
> in that function, in the preceding:
>
>   if (elided && !cands)
> {
>   error ("cannot deduce template arguments for copy-initialization"
>  " of %qT, as it has no non-explicit deduction guides or "
>  "user-declared constructors", type);
>   return error_mark_node;
> }
>
> and in the following:
>
>   if (elided)
> inform (input_location, "explicit deduction guides not considered "
> "for copy-initialization");
>
> and now I'm wondering which, if any, should be also removed?!? Both? Can you
> help me about that?
>
> Thanks,
> Paolo.

These messages are only emitted when elided == true, which, if I'm
reading the code correctly, is only the case when you are in
copy-initialization context (and have skipped at least one explicit
guide).

Re: [PATCH 2/3] Matching tokens: C parts (v2)

[Jeff Law]

On 08/04/2017 08:32 AM, David Malcolm wrote:
> On Thu, 2017-08-03 at 11:34 -0600, Jeff Law wrote:
>> On 08/01/2017 02:21 PM, David Malcolm wrote:
>>> Changed in v2:
>>>
>>> * Renamed template argument to traits_t; eliminated subclasses,
>>> just
>>>   using traits struct.
>>> * Moved enum constants into struct bodies (string constants can't
>>> be
>>>   without constexpr, which isn't available in C++98).
>>> * Fixed typo.
>>>
>>> OK for trunk?
>>>
>>> gcc/c/ChangeLog:
>>> * c-parser.c (c_parser_error): Rename to...
>>> (c_parser_error_richloc): ...this, making static, and adding
>>> "richloc" parameter, passing it to the c_parse_error call,
>>> rather than calling c_parser_set_source_position_from_token.
>>> (c_parser_error): Reintroduce, reimplementing in terms of the
>>> above, converting return type from void to bool.
>>> (class token_pair): New class.
>>> (struct matching_paren_traits): New struct.
>>> (matching_parens): New typedef.
>>> (struct matching_brace_traits): New struct.
>>> (matching_braces): New typedef.
>>> (get_matching_symbol): New function.
>>> (c_parser_require): Add param MATCHING_LOCATION, using it to
>>> highlight matching "opening" tokens for missing "closing"
>>> tokens.
>>> (c_parser_skip_until_found): Likewise.
>>> (c_parser_static_assert_declaration_no_semi): Convert explicit
>>> parsing of CPP_OPEN_PAREN and CPP_CLOSE_PAREN to use of
>>> class matching_parens, so that the pertinent open parenthesis
>>> is
>>> highlighted when there are problems locating the close
>>> parenthesis.
>>> (c_parser_struct_or_union_specifier): Likewise.
>>> (c_parser_typeof_specifier): Likewise.
>>> (c_parser_alignas_specifier): Likewise.
>>> (c_parser_simple_asm_expr): Likewise.
>>> (c_parser_braced_init): Likewise, for matching_braces.
>>> (c_parser_paren_condition): Likewise, for matching_parens.
>>> (c_parser_switch_statement): Likewise.
>>> (c_parser_for_statement): Likewise.
>>> (c_parser_asm_statement): Likewise.
>>> (c_parser_asm_operands): Likewise.
>>> (c_parser_cast_expression): Likewise.
>>> (c_parser_sizeof_expression): Likewise.
>>> (c_parser_alignof_expression): Likewise.
>>> (c_parser_generic_selection): Likewise.
>>> (c_parser_postfix_expression): Likewise for cases RID_VA_ARG,
>>> RID_OFFSETOF, RID_TYPES_COMPATIBLE_P, RID_AT_SELECTOR,
>>> RID_AT_PROTOCOL, RID_AT_ENCODE, reindenting as necessary.
>>> In case CPP_OPEN_PAREN, pass loc_open_paren to the
>>> c_parser_skip_until_found call.
>>> (c_parser_objc_class_definition): Use class matching_parens as
>>> above.
>>> (c_parser_objc_method_decl): Likewise.
>>> (c_parser_objc_try_catch_finally_statement): Likewise.
>>> (c_parser_objc_synchronized_statement): Likewise.
>>> (c_parser_objc_at_property_declaration): Likewise.
>>> (c_parser_oacc_wait_list): Likewise.
>>> (c_parser_omp_var_list_parens): Likewise.
>>> (c_parser_omp_clause_collapse): Likewise.
>>> (c_parser_omp_clause_default): Likewise.
>>> (c_parser_omp_clause_if): Likewise.
>>> (c_parser_omp_clause_num_threads): Likewise.
>>> (c_parser_omp_clause_num_tasks): Likewise.
>>> (c_parser_omp_clause_grainsize): Likewise.
>>> (c_parser_omp_clause_priority): Likewise.
>>> (c_parser_omp_clause_hint): Likewise.
>>> (c_parser_omp_clause_defaultmap): Likewise.
>>> (c_parser_oacc_single_int_clause): Likewise.
>>> (c_parser_omp_clause_ordered): Likewise.
>>> (c_parser_omp_clause_reduction): Likewise.
>>> (c_parser_omp_clause_schedule): Likewise.
>>> (c_parser_omp_clause_num_teams): Likewise.
>>> (c_parser_omp_clause_thread_limit): Likewise.
>>> (c_parser_omp_clause_aligned): Likewise.
>>> (c_parser_omp_clause_linear): Likewise.
>>> (c_parser_omp_clause_safelen): Likewise.
>>> (c_parser_omp_clause_simdlen): Likewise.
>>> (c_parser_omp_clause_depend): Likewise.
>>> (c_parser_omp_clause_map): Likewise.
>>> (c_parser_omp_clause_device): Likewise.
>>> (c_parser_omp_clause_dist_schedule): Likewise.
>>> (c_parser_omp_clause_proc_bind): Likewise.
>>> (c_parser_omp_clause_uniform): Likewise.
>>> (c_parser_omp_for_loop): Likewise.
>>> (c_parser_cilk_clause_vectorlength): Likewise.
>>> (c_parser_cilk_clause_linear): Likewise.
>>> (c_parser_transaction_expression): Likewise.
>>> * c-parser.h (c_parser_require): Add param matching_location
>>> with
>>> default UNKNOWN_LOCATION.
>>> (c_parser_error): Convert return type from void to bool.
>>> (c_parser_skip_until_found): Add param matching_location with
>>> default UNKNOWN_LOCATION.
>>>
>>> gcc/testsuite/ChangeLog:
>>> * gcc.dg/unclosed-init.c: New test case.
>>
>> Phew.  I only spot-checked most of the changes around the new API for
>> requiring the open/close paren/brace/bracket or consuming
>> parens/braces/brackets.  They were

Re: [C++ Patch] PR 79790 ("[C++17] ICE class template argument deduction")

[Paolo Carlini]

Hi,

On 04/08/2017 20:05, Tim Song wrote:

These messages are only emitted when elided == true, which, if I'm
reading the code correctly, is only the case when you are in
copy-initialization context (and have skipped at least one explicit
guide).

Ah! Thanks!

Paolo.

Re: [PATCH, rs6000] Move vec_mulo() and vec_mule() builtin test cases to P8 test file.

[Segher Boessenkool]

On Fri, Aug 04, 2017 at 10:27:29AM -0700, Carl Love wrote:
> The following patch moves the vec_mule() and vec_mulo() builtin test
> cases to the builtins-3-p8.c test case file.  These builtins use P8
> instructions so they need to be in the P8 test file.

Okay for trunk.  Thanks for fixing this!  One comment on the changelog
below.


> 2017-08-04  Carl Love  
> 
>   * gcc.target/powerpc/builtins-3.c: Remove builtin test cases for
>   vec_mule, and vec_mulo.
>   * gcc.target/powerpc/builtins-3-p8.c: Add builtin test cases for
>   vec_mule, and vec_mulo.

Maybe you could mention these are for the word variants?  The smaller
ones don't need ISA 2.07.


Segher

Re: [PATCH] Fix switch expansion (PR middle-end/81698)

[Richard Biener]

On August 4, 2017 5:42:21 PM GMT+02:00, Jakub Jelinek  wrote:
>Hi!
>
>As mentioned inthe PR, the switch expansion relies on EDGE_SUCC (bb, 0)
>from a GIMPLE_SWITCH to be the default edge, but GIMPLE only ensures
>that
>default label is the first one.
>
>So, this patch (in expand_case) instead finds the default_edge from the
>default label and corresponding block.
>Another issue is emit_case_dispatch_table - this function is called
>from
>expand_case as well as sjlj landing pad handling.  For the former, it
>suffers from a similar problem, and additionally we could have removed
>the default edge already, so even for switches that formerly had a
>default
>label we suddenly consider a random different edge as the default.
>For sjlj, I've just looked at one testcase in cross to
>hppa2.0w-hp-hpux10.0,
>and I believe stmt_bb should be just NULL, in any case if it would be
>non-NULL and there is some edge, it would hardly be the default edge,
>since the caller is emitting the default label after the switch
>expansion.
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

Richard.

>2017-08-04  Jakub Jelinek  
>
>   PR middle-end/81698
>   * stmt.c (emit_case_dispatch_table): Add DEFAULT_EDGE argument,
>   instead of computing it in the function.  Formatting fix.
>   (expand_case): Don't rely on default_edge being the first edge,
>   clear it if removing it, pass default_edge to
>   emit_case_dispatch_table.
>   (expand_sjlj_dispatch_table): Pass NULL as DEFAULT_EDGE, formatting
>   fix.
>
>--- gcc/stmt.c.jj  2017-07-14 13:04:47.0 +0200
>+++ gcc/stmt.c 2017-08-04 14:36:28.936447265 +0200
>@@ -945,8 +945,8 @@ conditional_probability (profile_probabi
> static void
> emit_case_dispatch_table (tree index_expr, tree index_type,
> struct case_node *case_list, rtx default_label,
>-tree minval, tree maxval, tree range,
>-  basic_block stmt_bb)
>+edge default_edge,  tree minval, tree maxval,
>+tree range, basic_block stmt_bb)
> {
>   int i, ncases;
>   struct case_node *n;
>@@ -954,7 +954,6 @@ emit_case_dispatch_table (tree index_exp
>   rtx_insn *fallback_label = label_rtx (case_list->code_label);
>   rtx_code_label *table_label = gen_label_rtx ();
>   bool has_gaps = false;
>-  edge default_edge = stmt_bb ? EDGE_SUCC (stmt_bb, 0) : NULL;
>profile_probability default_prob = default_edge ?
>default_edge->probability
> : profile_probability::never 
> ();
>   profile_probability base = get_outgoing_edge_probs (stmt_bb);
>@@ -1026,9 +1025,8 @@ emit_case_dispatch_table (tree index_exp
>  through the indirect jump or the direct conditional jump
>  before that. Split the probability of reaching the
>  default label among these two jumps.  */
>-  new_default_prob = conditional_probability
>(default_prob.apply_scale
>-   (1, 2),
>-  base);
>+  new_default_prob
>+  = conditional_probability (default_prob.apply_scale (1, 2), base);
>   default_prob = default_prob.apply_scale (1, 2);
>   base -= default_prob;
> }
>@@ -1147,9 +1145,10 @@ expand_case (gswitch *stmt)
>   do_pending_stack_adjust ();
> 
>   /* Find the default case target label.  */
>-  default_label = jump_target_rtx
>-  (CASE_LABEL (gimple_switch_default_label (stmt)));
>-  edge default_edge = EDGE_SUCC (bb, 0);
>+  tree default_lab = CASE_LABEL (gimple_switch_default_label (stmt));
>+  default_label = jump_target_rtx (default_lab);
>+  basic_block default_bb = label_to_block_fn (cfun, default_lab);
>+  edge default_edge = find_edge (bb, default_bb);
>   profile_probability default_prob = default_edge->probability;
> 
>   /* Get upper and lower bounds of case values.  */
>@@ -1245,9 +1244,10 @@ expand_case (gswitch *stmt)
>   {
> default_label = NULL;
> remove_edge (default_edge);
>+default_edge = NULL;
>   }
>   emit_case_dispatch_table (index_expr, index_type,
>-  case_list, default_label,
>+  case_list, default_label, default_edge,
>   minval, maxval, range, bb);
> }
> 
>@@ -1340,9 +1340,9 @@ expand_sjlj_dispatch_table (rtx dispatch
>   }
> 
>   emit_case_dispatch_table (index_expr, index_type,
>-  case_list, default_label,
>+  case_list, default_label, NULL,
>   minval, maxval, range,
>-BLOCK_FOR_INSN (before_case));
>+  BLOCK_FOR_INSN (before_case));
>   emit_label (default_label);
> }
> 
>
>   Jakub

Re: [PATCH 0/2] add unique_ptr class

[Jonathan Wakely]

On 31/07/17 19:46 -0400, tbsaunde+...@tbsaunde.org wrote:

I've been saying I'd do this for a long time, but I'm finally getting to
importing the C++98 compatable unique_ptr class Pedro wrote for gdb a while
back.  I believe the gtl namespace also comes from Pedro, but GNU template
library seems as reasonable as any other name I can come up with.


If it's inspired by "STL" then can we call it something else?

std::unique_ptr is not part of the STL, because the STL is a library
of containers and algorithms from the 1990s. std::unique_ptr is
something much newer that doesn't originate in the STL.

STL != C++ Standard Library

If we want a namespace for GNU utilities, what's wrong with "gnu"?

Re: [patch, gcc doc + fortran] Make -Ofast honor -fmax-stack-var-size

[Thomas Koenig]

Am 04.08.2017 um 14:09 schrieb Thomas Koenig:

Hello world,

the attached patch


This time, really attached, even with ChangeLog!

Regards

Thomas

2017-08-04  Thomas Koenig  

PR fortran/68829
* doc/invoke.texi: Document change in behvaior for -Ofast for
Fortran.

2017-08-04  Thomas Koenig  

PR fortran/68829
PR fortran/81701
* options.c: Make -Ofast honor -fmax-stack-var-size.
* invoke.texi: Document change.

2017-08-04  Thomas Koenig  

PR fortran/68829
PR fortran/81701
* gfortran.dg/o_fast_stacksize.90:  New test.



makes -Ofast honor -fmax-stack-var-size, and adjusts
the documentation in the gcc and fortran directories accordingly.
This is done to alleviate PR 68829, to make it possible to run -Ofast
with less stack usage.  I have also taken the opportunity to make
it a bit clearer what -fstack-arrays actually does.

I intend to close PR 68829 once this is committed.

Regression-tested, plus doc changes tested with "make pdf" and
"make dvi".

OK for trunk for

- the fortran subdirectory changes
- the gcc doc changes (if the one above is approved)

?

Regards

 Thomas



Index: doc/invoke.texi
===
--- doc/invoke.texi	(Revision 250720)
+++ doc/invoke.texi	(Arbeitskopie)
@@ -7278,7 +7278,8 @@
 @option{-O3} optimizations.  It also enables optimizations that are not
 valid for all standard-compliant programs.
 It turns on @option{-ffast-math} and the Fortran-specific
-@option{-fno-protect-parens} and @option{-fstack-arrays}.
+@option{-fstack-arrays}, unless @option{-fmax-stack-var-size} is
+specified, and @option{-fno-protect-parens}.
 
 @item -Og
 @opindex Og
Index: fortran/invoke.texi
===
--- fortran/invoke.texi	(Revision 250791)
+++ fortran/invoke.texi	(Arbeitskopie)
@@ -1570,13 +1570,13 @@
 
 @item -fstack-arrays
 @opindex @code{fstack-arrays}
-Adding this option will make the Fortran compiler put all local arrays,
-even those of unknown size onto stack memory.  If your program uses very
+Adding this option will make the Fortran compiler put all arrays of
+unknown size and array temporaries onto stack memory.  If your program uses very
 large local arrays it is possible that you will have to extend your runtime
 limits for stack memory on some operating systems. This flag is enabled
-by default at optimization level @option{-Ofast}.
+by default at optimization level @option{-Ofast} unless
+@option{-fmax-stack-var-size} is specified.
 
-
 @item -fpack-derived
 @opindex @code{fpack-derived}
 @cindex structure packing
Index: fortran/options.c
===
--- fortran/options.c	(Revision 250720)
+++ fortran/options.c	(Arbeitskopie)
@@ -235,7 +235,9 @@
   if (flag_protect_parens == -1)
 flag_protect_parens = !optimize_fast;
 
-  if (flag_stack_arrays == -1)
+  /* -Ofast sets implies -fstack-arrays unless an explicit size is set for
+ stack arrays.  */
+  if (flag_stack_arrays == -1 && flag_max_stack_var_size == -2)
 flag_stack_arrays = optimize_fast;
 
   /* By default, disable (re)allocation during assignment for -std=f95,
@@ -380,6 +382,10 @@
   flag_max_stack_var_size = -1;
 }
 
+  /* Set flag_stack_arrays correctly.  */
+  if (flag_stack_arrays == -1)
+flag_stack_arrays = 0;
+
   /* Set default.  */
   if (flag_max_stack_var_size == -2)
 flag_max_stack_var_size = 32768;
! { dg-do compile }
! { dg-options "-Ofast -fmax-stack-var-size=100 -fdump-tree-original" }
MODULE foo
CONTAINS
  SUBROUTINE mysum(a)
INTEGER :: a(:)
WRITE(6,*) SUM(a)
  END SUBROUTINE
END MODULE foo

USE foo
INTEGER, ALLOCATABLE :: a(:)
INTEGER, PARAMETER :: N=2**26 ! 256Mb array
ALLOCATE(a(N)) ; a=1
CALL mysum(a*a)
END
! { dg-final { scan-tree-dump-times "__builtin_malloc" 2 "original" } }

Re: [PATCH 1/2] add unique_ptr header

[Jonathan Wakely]

On 01/08/17 23:09 -0400, Trevor Saunders wrote:

aiui C++03 is C++98 with a few additions to the stl.


Again, STL != C++ Standard Library.

C++03 made some important changes to the core language, particularly
the value-initialization rules.

Most of the library changes were small bug fixes, and most were to
locales (which didn't originate in the STL and aren't even templates)
and iostreams (which didn't originate in the STL).

There were also changes to std::auto_ptr (also not from the STL) which
this might rely on (I haven't checked).

[PATCH][PING][PR target/81535] Fix tests on Power

[Yury Gribov]

Hi all,

This patch fixes issues reported in 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81535


I removed call to g in pr79439.c because gcc was duplicating the basic 
block with call depending on compiler flags (so scan-assembler-times 
pattern wasn't reliable anymore).  I also added alias to prevent 
inlining introduced by recent PR56727 patch.


I added Power-specific pattern in pr56727-2.c testcase and disabled 
testing on Power in pr56727-1.c.


Tested on powerpc64-unknown-linux-gnu.  Ok for trunk?

-Y

2017-07-28  Yury Gribov  

PR target/81535
* gcc.dg/pr56727-1.c: Do not check output on Power.
* gcc.dg/pr56727-2.c: Fix pattern for Power.
* gcc.target/powerpc/pr79439.c: Prevent inlining.

diff -rupN gcc/gcc/testsuite/gcc.dg/pr56727-1.c 
gcc-81535/gcc/testsuite/gcc.dg/pr56727-1.c
--- gcc/gcc/testsuite/gcc.dg/pr56727-1.c2017-07-28 02:39:54.770046466 
+
+++ gcc-81535/gcc/testsuite/gcc.dg/pr56727-1.c  2017-07-28 04:25:04.805648587 
+
@@ -1,6 +1,6 @@
 /* { dg-do compile { target fpic } } */
 /* { dg-options "-O2 -fPIC" } */
-/* { dg-final { scan-assembler-not "@(PLT|plt)" { target i?86-*-* x86_64-*-* 
powerpc*-*-* } } } */
+/* { dg-final { scan-assembler-not "@(PLT|plt)" { target i?86-*-* x86_64-*-* } 
} } */
 
 #define define_func(type) \
   void f_ ## type (type b) { f_ ## type (0); } \
diff -rupN gcc/gcc/testsuite/gcc.dg/pr56727-2.c 
gcc-81535/gcc/testsuite/gcc.dg/pr56727-2.c
--- gcc/gcc/testsuite/gcc.dg/pr56727-2.c2017-07-28 02:39:54.770046466 
+
+++ gcc-81535/gcc/testsuite/gcc.dg/pr56727-2.c  2017-07-28 04:21:19.195215187 
+
@@ -1,10 +1,10 @@
 /* { dg-do compile { target fpic } } */
 /* { dg-options "-O2 -fPIC" } */
-/* { dg-final { scan-assembler "@(PLT|plt)" { target i?86-*-* x86_64-*-* 
powerpc*-*-linux* } } } */
 
 __attribute__((noinline, noclone))
 void f (short b)
 {
+  __builtin_setjmp (0);  /* Prevent tailcall */
   f (0);
 }
 
@@ -14,3 +14,5 @@ void h ()
 {
   g (0);
 }
+/* { dg-final { scan-assembler "@(PLT|plt)" { target i?86-*-* x86_64-*-* } } } 
*/
+/* { dg-final { scan-assembler "bl f\n\[ \t\]*nop" { target powerpc*-*-linux* 
} } } */
diff -rupN gcc/gcc/testsuite/gcc.target/powerpc/pr79439.c 
gcc-81535/gcc/testsuite/gcc.target/powerpc/pr79439.c
--- gcc/gcc/testsuite/gcc.target/powerpc/pr79439.c  2017-07-28 
02:39:55.750048426 +
+++ gcc-81535/gcc/testsuite/gcc.target/powerpc/pr79439.c2017-07-28 
04:13:47.834177237 +
@@ -8,22 +8,17 @@
 
 int f (void);
 
-void
-g (void)
-{
-}
-
 int
 rec (int a)
 {
   int ret = 0;
   if (a > 10 && f ())
 ret += rec (a - 1);
-  g ();
   return a + ret;
 }
 
+void rec_alias (short) __attribute__ ((alias ("rec")));
+
 /* { dg-final { scan-assembler-times {\mbl f\M}   1 } } */
-/* { dg-final { scan-assembler-times {\mbl g\M}   1 } } */
 /* { dg-final { scan-assembler-times {\mbl rec\M} 1 } } */
-/* { dg-final { scan-assembler-times {\mnop\M}3 } } */
+/* { dg-final { scan-assembler-times {\mnop\M}2 } } */

[PATCH 02/22] libcpp: add linemap_position_for_file_line_and_column

[David Malcolm]

gcc/ChangeLog:
* input.c (selftest::test_making_arbitrary_locations): New function.
(selftest::input_c_tests): Call it.

libcpp/ChangeLog:
* include/line-map.h (linemap_position_for_file_line_and_column):
New decl.
* line-map.c (linemap_position_for_file_line_and_column): New
function.
---
 gcc/input.c   | 32 +
 libcpp/include/line-map.h |  9 +
 libcpp/line-map.c | 51 +++
 3 files changed, 92 insertions(+)

diff --git a/gcc/input.c b/gcc/input.c
index 1aad551..a3a8454 100644
--- a/gcc/input.c
+++ b/gcc/input.c
@@ -1795,6 +1795,37 @@ test_accessing_ordinary_linemaps (const line_table_case 
&case_)
   ASSERT_EQ (loc_d, src_range.m_finish);
 }
 
+/* Verify that linemap_position_for_file_line_and_column works.  */
+
+static void
+test_making_arbitrary_locations (const line_table_case &case_)
+{
+  line_table_test ltt (case_);
+
+  /* Verify that we can make various locations in arbitrary order,
+ sometimes changing file, sometimes going back and making a
+ location "earlier" than ones we've created before.  */
+
+  ASSERT_LOCEQ ("foo.c", 10, 5,
+   linemap_position_for_file_line_and_column (line_table,
+  "foo.c", 10, 5));
+  ASSERT_LOCEQ ("foo.c", 10, 6,
+   linemap_position_for_file_line_and_column (line_table,
+  "foo.c", 10, 6));
+  ASSERT_LOCEQ ("foo.c", 20, 1,
+   linemap_position_for_file_line_and_column (line_table,
+  "foo.c", 20, 1));
+  ASSERT_LOCEQ ("bar.c", 100, 12,
+   linemap_position_for_file_line_and_column (line_table,
+  "bar.c", 100, 12));
+  ASSERT_LOCEQ ("foo.c", 30, 1,
+   linemap_position_for_file_line_and_column (line_table,
+  "foo.c", 30, 1));
+  ASSERT_LOCEQ ("foo.c", 15, 1,
+   linemap_position_for_file_line_and_column (line_table,
+  "foo.c", 15, 1));
+}
+
 /* Verify various properties of UNKNOWN_LOCATION.  */
 
 static void
@@ -3528,6 +3559,7 @@ input_c_tests ()
   for_each_line_table_case (test_make_location_nonpure_range_endpoints);
 
   for_each_line_table_case (test_accessing_ordinary_linemaps);
+  for_each_line_table_case (test_making_arbitrary_locations);
   for_each_line_table_case (test_lexer);
   for_each_line_table_case (test_lexer_string_locations_simple);
   for_each_line_table_case (test_lexer_string_locations_ebcdic);
diff --git a/libcpp/include/line-map.h b/libcpp/include/line-map.h
index e696041..3c74bb0 100644
--- a/libcpp/include/line-map.h
+++ b/libcpp/include/line-map.h
@@ -1192,6 +1192,15 @@ linemap_position_for_loc_and_offset (struct line_maps 
*set,
 source_location loc,
 unsigned int offset);
 
+/* Encode and return a source location from a given file, line and column.
+   This is much less efficient than the above functions, and should only
+   be used as a last resort.  */
+
+source_location
+linemap_position_for_file_line_and_column (struct line_maps *set,
+  const char *, linenum_type,
+  unsigned int);
+
 /* Return the file this map is for.  */
 inline const char *
 LINEMAP_FILE (const line_map_ordinary *ord_map)
diff --git a/libcpp/line-map.c b/libcpp/line-map.c
index 0e5804b..32294f5 100644
--- a/libcpp/line-map.c
+++ b/libcpp/line-map.c
@@ -935,6 +935,57 @@ linemap_position_for_loc_and_offset (struct line_maps *set,
   return r;
 }
 
+/* Encode and return a source location from a given file, line and column.
+   This is much less efficient than the above functions, and should only
+   be used as a last resort.  */
+
+source_location
+linemap_position_for_file_line_and_column (struct line_maps *set,
+  const char *file, linenum_type line,
+  unsigned int column)
+{
+  /* First, attempt to find a pre-existing linemap that can represent
+ the location.  */
+  for (unsigned int i = 0; i < LINEMAPS_ORDINARY_USED (set); i++)
+{
+  line_map_ordinary *ord_map = LINEMAPS_ORDINARY_MAP_AT (set, i);
+  if (0 == strcmp (file, ord_map->to_file))
+   {
+ source_location loc
+   = linemap_position_for_line_and_column (set, ord_map,
+   line, column);
+ /* Check that it's a valid location within ord_map.  */
+ if (i + 1 < LINEMAPS_ORDINARY_USED (set))
+   {
+ line_map_ordinary *next_ord_map
+   = LINEMAPS_ORDINARY_MAP_AT (set, i + 1);
+ if (lo

[PATCH 01/22] Expose assert_loceq outside of input.c; add ASSERT_LOCEQ

[David Malcolm]

gcc/ChangeLog:
* input.c: Include "selftest-input.h".
(selftest::assert_loceq): Remove "static".  Add "report_loc" param
and update assertions to use it.
(selftest::test_accessing_ordinary_linemaps): Use ASSERT_LOCEQ
rather than assert_loceq.
(selftest::test_builtins): Likewise.
* selftest-input.h: New file.
---
 gcc/input.c  | 39 +++--
 gcc/selftest-input.h | 54 
 2 files changed, 75 insertions(+), 18 deletions(-)
 create mode 100644 gcc/selftest-input.h

diff --git a/gcc/input.c b/gcc/input.c
index 0480eb2..1aad551 100644
--- a/gcc/input.c
+++ b/gcc/input.c
@@ -23,6 +23,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "intl.h"
 #include "diagnostic-core.h"
 #include "selftest.h"
+#include "selftest-input.h"
 #include "cpplib.h"
 
 #ifndef HAVE_ICONV
@@ -1613,21 +1614,23 @@ test_should_have_column_data_p ()
 }
 
 /* Verify the result of LOCATION_FILE/LOCATION_LINE/LOCATION_COLUMN
-   on LOC.  */
+   on LOC.  Use REPORT_LOC as the effective location when reporting
+   any issues.  */
 
-static void
-assert_loceq (const char *exp_filename, int exp_linenum, int exp_colnum,
+void
+assert_loceq (const location &report_loc,
+ const char *exp_filename, int exp_linenum, int exp_colnum,
  location_t loc)
 {
-  ASSERT_STREQ (exp_filename, LOCATION_FILE (loc));
-  ASSERT_EQ (exp_linenum, LOCATION_LINE (loc));
+  ASSERT_STREQ_AT (report_loc, exp_filename, LOCATION_FILE (loc));
+  ASSERT_EQ_AT (report_loc, exp_linenum, LOCATION_LINE (loc));
   /* If location_t values are sufficiently high, then column numbers
  will be unavailable and LOCATION_COLUMN (loc) will be 0.
  When close to the threshold, column numbers *may* be present: if
  the final linemap before the threshold contains a line that straddles
  the threshold, locations in that line have column information.  */
   if (should_have_column_data_p (loc))
-ASSERT_EQ (exp_colnum, LOCATION_COLUMN (loc));
+ASSERT_EQ_AT (report_loc, exp_colnum, LOCATION_COLUMN (loc));
 }
 
 /* Various selftests involve constructing a line table and one or more
@@ -1761,23 +1764,23 @@ test_accessing_ordinary_linemaps (const line_table_case 
&case_)
   linemap_add (line_table, LC_LEAVE, false, NULL, 0);
 
   /* Verify that we can recover the location info.  */
-  assert_loceq ("foo.c", 1, 1, loc_a);
-  assert_loceq ("foo.c", 1, 23, loc_b);
-  assert_loceq ("foo.c", 2, 1, loc_c);
-  assert_loceq ("foo.c", 2, 17, loc_d);
-  assert_loceq ("foo.c", 3, 700, loc_e);
-  assert_loceq ("foo.c", 4, 100, loc_back_to_short);
+  ASSERT_LOCEQ ("foo.c", 1, 1, loc_a);
+  ASSERT_LOCEQ ("foo.c", 1, 23, loc_b);
+  ASSERT_LOCEQ ("foo.c", 2, 1, loc_c);
+  ASSERT_LOCEQ ("foo.c", 2, 17, loc_d);
+  ASSERT_LOCEQ ("foo.c", 3, 700, loc_e);
+  ASSERT_LOCEQ ("foo.c", 4, 100, loc_back_to_short);
 
   /* In the very wide line, the initial location should be fully tracked.  */
-  assert_loceq ("foo.c", 5, 2000, loc_start_of_very_long_line);
+  ASSERT_LOCEQ ("foo.c", 5, 2000, loc_start_of_very_long_line);
   /* ...but once we exceed LINE_MAP_MAX_COLUMN_NUMBER column-tracking should
  be disabled.  */
-  assert_loceq ("foo.c", 5, 0, loc_too_wide);
-  assert_loceq ("foo.c", 5, 0, loc_too_wide_2);
+  ASSERT_LOCEQ ("foo.c", 5, 0, loc_too_wide);
+  ASSERT_LOCEQ ("foo.c", 5, 0, loc_too_wide_2);
   /*...and column-tracking should be re-enabled for subsequent lines.  */
-  assert_loceq ("foo.c", 6, 10, loc_sane_again);
+  ASSERT_LOCEQ ("foo.c", 6, 10, loc_sane_again);
 
-  assert_loceq ("bar.c", 1, 150, loc_f);
+  ASSERT_LOCEQ ("bar.c", 1, 150, loc_f);
 
   ASSERT_FALSE (is_location_from_builtin_token (loc_a));
   ASSERT_TRUE (pure_location_p (line_table, loc_a));
@@ -1807,7 +1810,7 @@ test_unknown_location ()
 static void
 test_builtins ()
 {
-  assert_loceq (_(""), 0, 0, BUILTINS_LOCATION);
+  ASSERT_LOCEQ (_(""), 0, 0, BUILTINS_LOCATION);
   ASSERT_PRED1 (is_location_from_builtin_token, BUILTINS_LOCATION);
 }
 
diff --git a/gcc/selftest-input.h b/gcc/selftest-input.h
new file mode 100644
index 000..d56af36
--- /dev/null
+++ b/gcc/selftest-input.h
@@ -0,0 +1,54 @@
+/* Support for selftests of location handling.
+   Copyright (C) 2016-2017 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify it under
+the terms of the GNU General Public License as published by the Free
+Software Foundation; either version 3, or (at your option) any later
+version.
+
+GCC is distributed in the hope that it will be useful, but WITHOUT ANY
+WARRANTY; without even the implied warranty of MERCHANTABILITY or
+FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3.  If not see
+.  */
+
+#ifndef

[PATCH 00/22] RFC: integrated 3rd-party static analysis support

[David Malcolm]

This patch kit clearly isn't ready yet as-is (see e.g. the
"known unknowns" below), but I'm posting it now in the hope of
getting early feedback.

Summary
===

This patch kit provides an easy way to make integrate 3rd-party static
analysis tools into gcc, and have them:
(a) report through gcc's diagnostic subsystem, and
(b) "watermark" the generated binaries with queryable data on what checkers
were run, and what the results were.

Here's an example showing gcc running a bank of 3rd-party checkers on this
source file:

  #include 

  void test ()
  {
void *ptr_1;
void *ptr_2;

ptr_1 = malloc (64);
if (!ptr_1)
  return;
ptr_2 = malloc (64);
if (!ptr_2)
  return;

free (ptr_2);
free (ptr_1);
  }

via a simple command-line:

  $ ./xgcc -B. -c conditional-leak.c -Wrun-analyzers=policy.json
  conditional-leak.c:13:5: error: Potential leak of memory pointed to by 
'ptr_1' [clang-analyzer:Memory leak]
   return;
   ^
  conditional-leak.c:8:11: note: state 1 of 4: Memory is allocated
 ptr_1 = malloc (64);
 ^
  conditional-leak.c:9:7: note: state 2 of 4: Assuming 'ptr_1' is non-null
 if (!ptr_1)
 ^
  conditional-leak.c:12:7: note: state 3 of 4: Assuming 'ptr_2' is null
 if (!ptr_2)
 ^
  conditional-leak.c:13:5: note: state 4 of 4: Potential leak of memory pointed 
to by 'ptr_1'
   return;
   ^
  conditional-leak.c:13:0: error: Memory leak: ptr_1 [cppcheck:memleak]
   return;

Of the checkers, clang's static analyzer and cppcheck both identify the
memory leak; the former also identifies the control flow (the other
checkers didn't report anything).

The idea is to provide a mechanism to make it easy for developers and
projects to impose policy on what checkers should be run, and to gate
the build if certain tests fail.

In this case, the results are treated as hard errors and block the build,
but policy could allow them to be warnings.

Extensive metadata is captured about what checkers were run, and what
they emitted, using the "Firehose" interchange format:

  http://firehose.readthedocs.io/en/latest/index.html

In the case where this doesn't block the build, this can be queried via a
  contrib/get-static-analysis.py
script, so e.g. you can verify that a setuid binary was indeed compiled
using all the checkers that you expect it to be.

This can also be used to embed data about the code into the watermark.
For example, checkers/ianal.py embeds information about "Copyright"
lines in the source code into the generated binaries, from where it
can be queried (this example is intended as a proof-of-concept rather
than as a real license-tracking solution...)


Statement of the problem


Static analysis is IMHO done too late, if at all: static analysis tools are run
as an optional extra, "on the side", rather than in developers' normal
workflow, with some kind of "override the compiler and do extra work" hook,
which may preclude running more than one analyzer at once.  Analysis results
are reviewed (if at all) in some kind of on-the-side tool, rather than when the
code is being edited, or patches being prepared.

It would be better to have an easy way for developers to run analyzer(s)
as they're doing development, as part of their edit-compile-test cycle
- analysis problems are reported immediately, and can be acted on
immediately (e.g. by treating some checker tests as being hard errors).

It would also be good to have a way to run analyzer(s) when packages are
built, with a variety of precanned policies for analyzers.  For example,
setuid binaries and network-facing daemons could each be built with a
higher strictness of checking.

It would also be good to tag binaries with information on what analyzers
were run, what options they were invoked with, etc.
Potentially have "dump_file" information from optimization passes stored
in the metadata also.   Have a tool to query all of this.

This way a distribution can perform a query like:

  "show me all setuid binaries that contain code that wasn't checked
   with $CHECKER with $TEST set to be a hard error"

Can/should we break the build if there are issues?
Yes: but have a way to opt-in easily: if the tool is well-integrated with the
compiler: e.g.
-Wrun-analyzers=/usr/share/analyzers/userspace/network-facing-service
then upstream developers and packagers can turn on the setting, and see what
breaks, and fix it naturally within an compile-edit-test cycle

This gives a relatively painless way to opt-in to increasing levels of
strictness (e.g. by an upstream project, or by an individual developer).

Does this slow the build down?
Yes: but you can choose which analyzers run, and can choose to turn them off.
It ought to parallelize well.  I believe users will prefer to turn them on,
and have builders burn up the extra CPU cycles.
This may make much more sense for binary distributions (e.g. Fedora, Debian)
that it does for things like Ge

[PATCH 03/22] Add JSON implementation

[David Malcolm]

This patch adds support to gcc for reading and writing JSON,
based on DOM-like trees of json::value instances.

gcc/ChangeLog:
* Makefile.in (OBJS): Add json.o.
* json.cc: New file.
* json.h: New file.
* selftest-run-tests.c (selftest::run_tests): Call json_cc_tests.
* selftest.h (selftest::json_cc_tests): New decl.
---
 gcc/Makefile.in  |1 +
 gcc/json.cc  | 1914 ++
 gcc/json.h   |  214 ++
 gcc/selftest-run-tests.c |1 +
 gcc/selftest.h   |1 +
 5 files changed, 2131 insertions(+)
 create mode 100644 gcc/json.cc
 create mode 100644 gcc/json.h

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index efca916..4f7fd0c 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -1368,6 +1368,7 @@ OBJS = \
ira-color.o \
ira-emit.o \
ira-lives.o \
+   json.o \
jump.o \
langhooks.o \
lcm.o \
diff --git a/gcc/json.cc b/gcc/json.cc
new file mode 100644
index 000..e0d5a76
--- /dev/null
+++ b/gcc/json.cc
@@ -0,0 +1,1914 @@
+/* JSON parsing
+   Copyright (C) 2017 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify it under
+the terms of the GNU General Public License as published by the Free
+Software Foundation; either version 3, or (at your option) any later
+version.
+
+GCC is distributed in the hope that it will be useful, but WITHOUT ANY
+WARRANTY; without even the implied warranty of MERCHANTABILITY or
+FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3.  If not see
+.  */
+
+#include "config.h"
+#include "system.h"
+#include "coretypes.h"
+#include "json.h"
+#include "pretty-print.h"
+#include "math.h"
+#include "selftest.h"
+
+using namespace json;
+
+/* class json::value.  */
+
+/* Generate a char * for this json::value tree.
+   The returned value must be freed by the caller.  */
+
+char *
+value::to_str () const
+{
+  pretty_printer pp;
+  print (&pp);
+  return xstrdup (pp_formatted_text (&pp));
+}
+
+/* Dump this json::value tree to OUTF.
+   No formatting is done.  There are no guarantees about the order
+   in which the key/value pairs of json::objects are printed.  */
+
+void
+value::dump (FILE *outf) const
+{
+  pretty_printer pp;
+  pp_buffer (&pp)->stream = outf;
+  print (&pp);
+  pp_flush (&pp);
+}
+
+/* If this json::value is a json::object, return it,
+   otherwise return NULL.  */
+
+const object *
+value::as_object () const
+{
+  if (get_kind () != JSON_OBJECT)
+return NULL;
+  return static_cast  (this);
+}
+
+/* If this json::value is a json::array, return it,
+   otherwise return NULL.  */
+
+const array *
+value::as_array () const
+{
+  if (get_kind () != JSON_ARRAY)
+return NULL;
+  return static_cast  (this);
+}
+
+/* If this json::value is a json::number, return it,
+   otherwise return NULL.  */
+
+const number *
+value::as_number () const
+{
+  if (get_kind () != JSON_NUMBER)
+return NULL;
+  return static_cast  (this);
+}
+
+/* If this json::value is a json::string, return it,
+   otherwise return NULL.  */
+
+const string *
+value::as_string () const
+{
+  if (get_kind () != JSON_STRING)
+return NULL;
+  return static_cast  (this);
+}
+
+/* Attempt to get the value of a key/value pair from this value
+   as if THIS value were an object.
+
+   If THIS is not a json::object, return write an error message to OUT_ERR
+   (which must be freed by the caller) and return false.
+
+   Otherwise write the value ptr (possibly NULL) to OUT_VALUE and
+   return true.  */
+
+bool
+value::get_optional_value_by_key (const char *name, const value *&out_value,
+ char *&out_err) const
+{
+  const json::object *obj = as_object ();
+  if (!obj)
+{
+  out_err = xstrdup ("not an object");
+  return false;
+}
+  out_value = obj->get (name);
+  return true;
+}
+
+/* Attempt to get a string value of a key/value pair from this value
+   as if THIS value were an object.
+
+   If THIS is a json::object, and KEY is either not present, is a string,
+   or is the "null" JSON literal, then return true, and write to OUT_VALUE.
+   If a string, then the ptr is written to OUT_VALUE, otherwise NULL
+   is written to OUT_VALUE.
+
+   If THIS is not a json::object, or KEY is not a string/"null",
+   return false and write an error message to OUT_ERR
+   (which must be freed by the caller).  */
+
+bool
+value::get_optional_string_by_key (const char *name, const char *&out_value,
+  char *&out_err) const
+{
+  const json::value *v;
+  if (!get_optional_value_by_key (name, v, out_err))
+return false;
+  if (v && v->get_kind () != JSON_NULL)
+{
+  const json::string *s = v->as_string ();
+  

[PATCH 17/22] Add checkers/cppcheck.py

[David Malcolm]

This patch adds a harness for invoking cppcheck:
   http://cppcheck.sourceforge.net/
returning the results in JSON format.

It runs "cppcheck --xml --xml-version=2", then uses
firehose.parsers.cppcheck.parse_file to parse the generated .xml file,
turning it into firehose JSON.

checkers/ChangeLog:
* cppcheck.py: New file.
---
 checkers/cppcheck.py | 138 +++
 1 file changed, 138 insertions(+)
 create mode 100755 checkers/cppcheck.py

diff --git a/checkers/cppcheck.py b/checkers/cppcheck.py
new file mode 100755
index 000..9b6a864
--- /dev/null
+++ b/checkers/cppcheck.py
@@ -0,0 +1,138 @@
+#!/usr/bin/env python
+#   Copyright 2012, 2013, 2015, 2017 David Malcolm 
+#   Copyright 2012, 2013, 2015, 2017 Red Hat, Inc.
+#
+#   This is free software: you can redistribute it and/or modify it
+#   under the terms of the GNU General Public License as published by
+#   the Free Software Foundation, either version 3 of the License, or
+#   (at your option) any later version.
+#
+#   This program is distributed in the hope that it will be useful, but
+#   WITHOUT ANY WARRANTY; without even the implied warranty of
+#   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+#   General Public License for more details.
+#
+#   You should have received a copy of the GNU General Public License
+#   along with this program.  If not, see
+#   .
+
+import sys
+import tempfile
+
+from firehose.model import Failure, Issue
+from firehose.parsers.cppcheck import parse_file
+from gccinvocation import GccInvocation
+
+from checker import Checker, CheckerTests, make_file, make_stats, \
+tool_main
+
+class InvokeCppcheck(Checker):
+"""
+Checker subclass that invokes "cppcheck"
+"""
+name = 'cppcheck'
+
+def raw_invoke(self, gccinv, sourcefile):
+args = ['cppcheck',
+'--xml', '--xml-version=2',
+sourcefile]
+return self._run_subprocess(sourcefile, args)
+
+def handle_output(self, result):
+if result.returncode:
+analysis = self._make_failed_analysis(result.sourcefile, 
result.timer,
+  msgtext='Bad exit code 
running %s' % self.name,
+  failureid='bad-exit-code')
+self.set_custom_fields(result, analysis)
+return analysis
+
+# (there doesn't seem to be a way to have cppcheck directly
+# save its XML output to a given location)
+
+with tempfile.NamedTemporaryFile() as outfile:
+outfile.write(result.err)
+outfile.flush()
+
+with open(outfile.name) as infile:
+# Parse stderr into firehose XML format and save:
+analysis = parse_file(infile,
+  file_=make_file(result.sourcefile),
+  stats=make_stats(result.timer))
+self.set_custom_fields(result, analysis)
+return analysis
+
+def set_custom_fields(self, result, analysis):
+analysis.set_custom_field('cppcheck-invocation',
+  ' '.join(result.argv))
+result.set_custom_fields(analysis)
+
+class CppcheckTests(CheckerTests):
+def make_tool(self):
+return self.make_tool_from_class(InvokeCppcheck)
+
+def verify_basic_metadata(self, analysis, sourcefile):
+# Verify basic metadata:
+self.assert_metadata(analysis, 'cppcheck', sourcefile)
+self.assert_has_custom_field(analysis, 'cppcheck-invocation')
+self.assert_has_custom_field(analysis, 'stdout')
+self.assert_has_custom_field(analysis, 'stderr')
+
+def test_file_not_found(self):
+analysis = self.invoke('does-not-exist.c')
+self.assertEqual(len(analysis.results), 1)
+self.assertIsInstance(analysis.results[0], Failure)
+self.assertEqual(analysis.results[0].failureid, 'bad-exit-code')
+
+def test_timeout(self):
+sourcefile = 'test-sources/harmless.c'
+tool = self.make_tool()
+tool.timeout = 0
+gccinv = GccInvocation(['gcc', sourcefile])
+analysis = tool.checked_invoke(gccinv, sourcefile)
+self.assert_metadata(analysis, 'cppcheck', sourcefile)
+self.assertEqual(len(analysis.results), 1)
+r0 = analysis.results[0]
+self.assertIsInstance(r0, Failure)
+self.assertEqual(r0.failureid, 'timeout')
+self.assert_has_custom_field(analysis, 'timeout')
+self.assert_has_custom_field(analysis, 'command-line')
+
+def test_harmless_file(self):
+analysis = self.invoke('test-sources/harmless.c')
+self.assertEqual(len(analysis.results), 0)
+
+def test_read_through_null(self):
+analysis = self.invoke('test-sources/read-through-null.c')
+self.assertEqual(len(analysis.results), 1)
+r0 = an

[PATCH 08/22] Add GNU_BUILD_ATTRIBUTE_STATIC_ANALYSIS to annobin.h

[David Malcolm]

gcc/ChangeLog:
* annobin.h (GNU_BUILD_ATTRIBUTE_STATIC_ANALYSIS): New define.
---
 gcc/annobin.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/gcc/annobin.h b/gcc/annobin.h
index 76eb01c..1152316 100644
--- a/gcc/annobin.h
+++ b/gcc/annobin.h
@@ -35,6 +35,7 @@
 #define GNU_BUILD_ATTRIBUTE_ABI6
 #define GNU_BUILD_ATTRIBUTE_PIC7
 #define GNU_BUILD_ATTRIBUTE_SHORT_ENUM 8
+#define GNU_BUILD_ATTRIBUTE_STATIC_ANALYSIS9
 
 #define NOTE_GNU_PROPERTY_SECTION_NAME ".note.gnu.property"
 #define GNU_BUILD_ATTRS_SECTION_NAME   ".gnu.build.attributes"
-- 
1.8.5.3

[PATCH 19/22] Add checkers/ianal.py

[David Malcolm]

This patch is a demo of handling code metrics and metadata ("info" results
in Firehose terminology).

It adds a standalone tool harness which scans the main input file
looking for "Copyright" lines, returning information on them as
firehose JSON.

When sent through GCC's diagnostic subsystem by checker.cc, these
"info" results are emitted as notes, e.g.:

../../src/checkers/test-sources/cpychecker-demo.c:2:4: note: I am not a lawyer 
[not-a-lawyer:copyright-line]
Copyright 2011 David Malcolm 
^
../../src/checkers/test-sources/cpychecker-demo.c:3:4: note: I am not a lawyer 
[not-a-lawyer:copyright-line]
Copyright 2011 Red Hat, Inc.
^

and they're captured in the generated binary by the watermarking code.

checkers/ChangeLog:
* ianal.py: New file.
---
 checkers/ianal.py | 79 +++
 1 file changed, 79 insertions(+)
 create mode 100755 checkers/ianal.py

diff --git a/checkers/ianal.py b/checkers/ianal.py
new file mode 100755
index 000..a918f41
--- /dev/null
+++ b/checkers/ianal.py
@@ -0,0 +1,79 @@
+#!/usr/bin/env python
+#   Copyright 2017 David Malcolm 
+#   Copyright 2017 Red Hat, Inc.
+#
+#   This is free software: you can redistribute it and/or modify it
+#   under the terms of the GNU General Public License as published by
+#   the Free Software Foundation, either version 3 of the License, or
+#   (at your option) any later version.
+#
+#   This program is distributed in the hope that it will be useful, but
+#   WITHOUT ANY WARRANTY; without even the implied warranty of
+#   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+#   General Public License for more details.
+#
+#   You should have received a copy of the GNU General Public License
+#   along with this program.  If not, see
+#   .
+
+import re
+import sys
+
+from firehose.model import Analysis, Generator, Metadata, Info, \
+Location, Message, Range, Point
+
+from checker import Checker, CheckerTests, make_file, tool_main
+
+class NotALawyer(Checker):
+"""
+Checker subclass that looks for "Copyright" lines, as a demo
+of handling "info" results.
+"""
+name = 'not-a-lawyer'
+
+def raw_invoke(self, gccinv, sourcefile):
+results = []
+file_ = make_file(sourcefile)
+with open(sourcefile) as f:
+for lineidx, line in enumerate(f):
+m = re.match('.*(Copyright).*', line)
+if m:
+start, end = m.span(1)
+linenum = lineidx + 1
+range_ = Range(start=Point(linenum, start + 1),
+   end=Point(linenum, end))
+location = Location(file_, None, range_=range_)
+info = Info(infoid='copyright-line',
+location=location,
+message=Message('I am not a lawyer'),
+customfields=None)
+results.append(info)
+metadata = Metadata(generator=Generator(self.name), sut=None,
+file_=file_, stats=None)
+analysis = Analysis(metadata, results)
+return analysis
+
+class NotALawyerTests(CheckerTests):
+def make_tool(self):
+return self.make_tool_from_class(NotALawyer)
+
+def verify_basic_metadata(self, analysis, sourcefile):
+# Verify basic metadata:
+self.assert_metadata(analysis, 'not-a-lawyer', sourcefile)
+
+def test_basic(self):
+analysis = self.invoke('test-sources/cpychecker-demo.c')
+self.assertEqual(len(analysis.results), 2)
+r0 = analysis.results[0]
+self.assertIsInstance(r0, Info)
+self.assertEqual(r0.infoid, 'copyright-line')
+self.assertEqual(r0.location.file.givenpath,
+ 'test-sources/cpychecker-demo.c')
+self.assertEqual(r0.message.text, 'I am not a lawyer')
+self.assertEqual(r0.location.range_.start.line, 2)
+self.assertEqual(r0.location.range_.start.column, 4)
+self.assertEqual(r0.location.range_.end.line, 2)
+self.assertEqual(r0.location.range_.end.column, 12)
+
+if __name__ == '__main__':
+sys.exit(tool_main(sys.argv, NotALawyer))
-- 
1.8.5.3

[PATCH 07/22] Add minimal version of Nick Clifton's annobin code

[David Malcolm]

This patch provides a way to "watermark" binaries with
metadata.  It's used later in the patch kit to watermark
binaries with static analysis results and metadata.

See:
  https://fedoraproject.org/wiki/Toolchain/Watermark

Note: this is a version of Nick Clifton's "annobin" gcc plugin:
  https://nickc.fedorapeople.org/
heavily hacked up by me:
* removed everything (including plugin support) not needed by
  later patches in the kit
* rewritten as an API, rather than as a plugin
* removed annobin_inform (..., "ICE: ...") calls in favor of
  gcc_assert.
* line-wrapped
* added a annobin_ensure_init to initialize annobin_is_64bit.
* added #ifndef guard to annobin.h

It includes the commits:
* Remove size limit on string passed to annobin_output_string_note
* Version 2 of spec: Add a GA prefix to all names

gcc/ChangeLog:
* Makefile.in (OBJS): Add annobin.o.
* annobin.cc: New file.
* annobin.h: New file.
---
 gcc/Makefile.in |   1 +
 gcc/annobin.cc  | 185 
 gcc/annobin.h   |  44 ++
 3 files changed, 230 insertions(+)
 create mode 100644 gcc/annobin.cc
 create mode 100644 gcc/annobin.h

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index 9ceb3f3..319e3f3 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -1216,6 +1216,7 @@ OBJS = \
ggc-page.o \
alias.o \
alloc-pool.o \
+   annobin.o \
auto-inc-dec.o \
auto-profile.o \
bb-reorder.o \
diff --git a/gcc/annobin.cc b/gcc/annobin.cc
new file mode 100644
index 000..ad8e49a
--- /dev/null
+++ b/gcc/annobin.cc
@@ -0,0 +1,185 @@
+/* annobin - support for annotating binary files.
+   Copyright (c) 2017 Red Hat.
+   Created by Nick Clifton.
+   Heavily hacked up by David Malcolm.
+
+  This is free software; you can redistribute it and/or modify it
+  under the terms of the GNU General Public License as published
+  by the Free Software Foundation; either version 3, or (at your
+  option) any later version.
+
+  It is distributed in the hope that it will be useful, but
+  WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+  GNU General Public License for more details.  */
+
+#include "config.h"
+#include "system.h"
+#include "coretypes.h"
+#include "tm.h"
+#include "diagnostic-core.h"
+#include "annobin.h"
+#include "output.h"
+
+/* Internal variable, used by target specific parts of the annobin plugin as 
well
+   as this generic part.  True if the object file being generated is for a 
64-bit
+   target.  */
+bool   annobin_is_64bit = false;
+
+static void
+annobin_ensure_init (void)
+{
+  static bool done_once = false;
+  if (done_once)
+return;
+  done_once = true;
+
+  /* Compute the default data size.  */
+  switch (POINTER_SIZE)
+{
+case 16:
+case 32:
+  annobin_is_64bit = false; break;
+case 64:
+  annobin_is_64bit = true; break;
+default:
+  sorry ("unknown target pointer size: %d", POINTER_SIZE);
+}
+}
+
+void
+annobin_output_note (const void * name, unsigned namesz, bool name_is_string,
+const char * name_description,
+const void * desc, unsigned descsz, bool desc_is_string,
+unsigned type)
+{
+  annobin_ensure_init ();
+
+  unsigned i;
+
+  if (type == NT_GNU_BUILD_ATTRIBUTE_FUNC
+  || type == NT_GNU_BUILD_ATTRIBUTE_OPEN)
+{
+  fprintf (asm_out_file, "\t.pushsection %s\n",
+  GNU_BUILD_ATTRS_SECTION_NAME);
+}
+
+  if (name == NULL)
+{
+  gcc_assert (namesz == 0);
+  fprintf (asm_out_file, "\t.dc.l 0\t\t%s no name\n", ASM_COMMENT_START);
+}
+  else if (name_is_string)
+{
+  gcc_assert (strlen ((const char *) name) == namesz - 1);
+  fprintf (asm_out_file, "\t.dc.l %u \t%s namesz = strlen (%s)\n", namesz,
+   ASM_COMMENT_START, (const char *) name);
+}
+  else
+fprintf (asm_out_file, "\t.dc.l %u\t\t%s size of name\n", namesz,
+ASM_COMMENT_START);
+
+  if (desc == NULL)
+{
+  gcc_assert (descsz == 0);
+  fprintf (asm_out_file, "\t.dc.l 0\t\t%s no description\n",
+  ASM_COMMENT_START);
+}
+  else if (desc_is_string)
+{
+  gcc_assert (descsz == (annobin_is_64bit ? 8 : 4));
+  fprintf (asm_out_file, "\t.dc.l %u\t\t%s descsz = sizeof (address)\n",
+  descsz, ASM_COMMENT_START);
+}
+  else
+fprintf (asm_out_file, "\t.dc.l %u\t\t%s size of description\n", descsz,
+ASM_COMMENT_START);
+
+  fprintf (asm_out_file, "\t.dc.l %#x\t%s type = %s\n", type, 
ASM_COMMENT_START,
+  type == NT_GNU_BUILD_ATTRIBUTE_OPEN ? "OPEN" :
+  type == NT_GNU_BUILD_ATTRIBUTE_FUNC ? "FUNC" :
+  type == NT_GNU_PROPERTY_TYPE_0  ? "PROPERTY_TYPE_0"
+  : "*UNKNOWN*");
+
+  if (name)
+{
+  if (name_is_string)
+   {
+ fprintf (asm_out_file, "\t.asciz \"%s\"", (const 

[PATCH 10/22] Add checkers.h/cc

[David Malcolm]

This implements a new "policy" class to read a description of
a set of checkers to run, along with a "checker" class to
handle actually running the checkers, ouputting the results
through gcc's diagnostic subsystem, and watermarking the
generated binary with the results and metadata.

Caveats:

* there's currently no way to express suppressions (e.g.
 "run clang-analyzer, but ignore errors foo, bar, and baz");
  that said, it *does* capture that metadata about the diagnostics.
  I'm thinking of something like a 4-state enum value per test id:
  - error: hard error that fails the build
  - warn: warn, but don't fail the build
  - log: capture within watermark, but don't warn
  - drop: ignore altogether
  defaulting to "error".

* the policy is read from a monolithic JSON file; this format
  is clunky for users to work with, and probably would be
  easier to do as simple gcc options, one per checker

* to save time, some of this code relies on stuff within
  "selftest", which would need moving out of there for
  release builds

* there are quite a few other FIXMEs in this patch

gcc/ChangeLog:
* Makefile.in (OBJS): Add checkers.o.
* checkers.cc: New file.
* checkers.h: New file.
* selftest-run-tests.c (selftest::run_tests): Call
selftest::checkers_cc_tests.
* selftest.h (selftest::checkers_cc_tests): New decl.

gcc/testsuite/ChangeLog:
* selftests/checker-policy/test-policy.json: New file.
---
 gcc/Makefile.in|   1 +
 gcc/checkers.cc| 736 +
 gcc/checkers.h |  26 +
 gcc/selftest-run-tests.c   |   1 +
 gcc/selftest.h |   1 +
 .../selftests/checker-policy/test-policy.json  |   7 +
 6 files changed, 772 insertions(+)
 create mode 100644 gcc/checkers.cc
 create mode 100644 gcc/checkers.h
 create mode 100644 gcc/testsuite/selftests/checker-policy/test-policy.json

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index 319e3f3..189833e 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -1236,6 +1236,7 @@ OBJS = \
cfgloopanal.o \
cfgloopmanip.o \
cfgrtl.o \
+   checkers.o \
symtab.o \
cgraph.o \
cgraphbuild.o \
diff --git a/gcc/checkers.cc b/gcc/checkers.cc
new file mode 100644
index 000..1a16799
--- /dev/null
+++ b/gcc/checkers.cc
@@ -0,0 +1,736 @@
+/* Running 3rd-party code analysis tools.
+   Copyright (C) 2017 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify it under
+the terms of the GNU General Public License as published by the Free
+Software Foundation; either version 3, or (at your option) any later
+version.
+
+GCC is distributed in the hope that it will be useful, but WITHOUT ANY
+WARRANTY; without even the implied warranty of MERCHANTABILITY or
+FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3.  If not see
+.  */
+
+#include "config.h"
+#include "system.h"
+#include "coretypes.h"
+#include "options.h"
+#include "diagnostic.h"
+#include "selftest.h"
+#include "firehose.h"
+#include "json.h"
+#include 
+#include "checkers.h"
+#include "annobin.h"
+#include "cpplib.h"
+#include "incpath.h"
+
+
+static bool
+diagnostic_at_rich_loc_va (rich_location *richloc,
+  diagnostic_info *diagnostic,
+  const char *gmsgid,
+  va_list *ap) ATTRIBUTE_GCC_DIAG(3,0);
+
+static bool
+diagnostic_at (location_t loc, diagnostic_info *diagnostic,
+  const char *gmsgid, ...)  ATTRIBUTE_GCC_DIAG(3,4);
+
+/* Print any trace of states associated with ISSUE.  */
+
+static void
+print_any_trace (const firehose::issue &issue)
+{
+  if (!issue.m_trace)
+return;
+  if (0)
+inform (UNKNOWN_LOCATION, "got trace");
+
+  /* Filter out any states within the trace that don't have text.  */
+  issue.m_trace->filter ();
+
+  /* If we're just left with a single state that duplicates what we
+ already printed for the issue, don't bother printing it.  */
+  if (issue.m_trace->is_redundant_p (issue))
+return;
+
+  int i;
+  firehose::state *s;
+  int num_states = issue.m_trace->m_states.length ();
+  FOR_EACH_VEC_ELT (issue.m_trace->m_states, i, s)
+{
+  if (s->m_notes)
+   inform (s->m_location, "state %i of %i: %s", i + 1, num_states,
+   s->m_notes);
+  else
+   inform (s->m_location, "state %i of %i", i + 1, num_states);
+}
+}
+
+/* FIXME.  */
+
+bool
+diagnostic_at_rich_loc_va (rich_location *richloc,
+  diagnostic_info *diagnostic,
+  const char *gmsgid,
+  va_list *ap)
+{
+  gcc_assert (rich

[PATCH 22/22] Add contrib/get-static-analysis.py

[David Malcolm]

This patch adds a simple Python 2/3 script for reading the
static analysis "watermark" from object files, writing the
JSON to stdout (prettyprinting it with indentation and newlines
for ease of human reading).

contrib/ChangeLog:
* get-static-analysis.py: New file.
---
 contrib/get-static-analysis.py | 47 ++
 1 file changed, 47 insertions(+)
 create mode 100644 contrib/get-static-analysis.py

diff --git a/contrib/get-static-analysis.py b/contrib/get-static-analysis.py
new file mode 100644
index 000..c246718
--- /dev/null
+++ b/contrib/get-static-analysis.py
@@ -0,0 +1,47 @@
+# FIXME
+# Extract static analysis results from input file and pretty-print JSON to 
stdout
+# This file is intended to be compatible with both Python 2 and Python 3
+
+import json
+import os
+import subprocess
+import sys
+import tempfile
+
+SECTION_NAME = '.gnu.build.attributes'
+
+def get_json_content(filename):
+"""
+Extract the JSON from SECTION_NAME from filename, returning
+as a bytes instance.
+"""
+with tempfile.NamedTemporaryFile() as outfile:
+try:
+subprocess.check_call(['objcopy', '-O', 'binary',
+   '--only-section=%s' % SECTION_NAME,
+   '--set-section-flags',
+   '%s=alloc' % SECTION_NAME,
+   filename, outfile.name])
+except subprocess.CalledProcessError:
+if not os.path.exists(outfile.name):
+outfile.delete = False
+raise
+with open(outfile.name, 'rb') as f_in:
+buf = f_in.read()
+if not buf:
+raise ValueError('section not found: %s' % SECTION_NAME)
+# Expect 16 bytes of header, then JSON, then a 0-terminator and 
padding
+json_buf = buf[16:].split(b'\x00')[0]
+return json_buf
+
+filename = sys.argv[1]
+try:
+json_buf = get_json_content(filename)
+except subprocess.CalledProcessError:
+sys.exit(1)
+except ValueError as exc:
+print(exc)
+sys.exit(1)
+json_str = json_buf.decode('utf-8')
+jv = json.loads(json_str)
+json.dump(jv, sys.stdout, indent=4)
-- 
1.8.5.3

[PATCH 09/22] Add selftest::read_file (..., FILE *, ...)

[David Malcolm]

This patch is a hack used by the followup checkers.cc patch,
and ought to be removed in any final version of the kit.

gcc/ChangeLog:
* selftest.c (read_file): New overload.
* selftest.h (read_file): New overload.
---
 gcc/selftest.c | 16 +---
 gcc/selftest.h |  7 +++
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/gcc/selftest.c b/gcc/selftest.c
index b41b9f5..a6674be 100644
--- a/gcc/selftest.c
+++ b/gcc/selftest.c
@@ -162,7 +162,19 @@ read_file (const location &loc, const char *path)
   FILE *f_in = fopen (path, "r");
   if (!f_in)
 fail_formatted (loc, "unable to open file: %s", path);
+  char *result = read_file (loc, f_in, path);
+  fclose (f_in);
+  return result;
+}
+
+/* Read all of F_IN into memory, returning a 0-terminated buffer
+   that must be freed by the caller.  F_IN is *not* closed.
+   Fail (and abort) if there are any problems, with LOC as the reported
+   location of the failure, using DESC as a description of the file.  */
 
+char *
+read_file (const location &loc, FILE *f_in, const char *desc)
+{
   /* Read content, allocating FIXME.  */
   char *result = NULL;
   size_t total_sz = 0;
@@ -186,11 +198,9 @@ read_file (const location &loc, const char *path)
 }
 
   if (!feof (f_in))
-fail_formatted (loc, "error reading from %s: %s", path,
+fail_formatted (loc, "error reading from %s: %s", desc,
xstrerror (errno));
 
-  fclose (f_in);
-
   /* 0-terminate the buffer.  */
   gcc_assert (total_sz < alloc_sz);
   result[total_sz] = '\0';
diff --git a/gcc/selftest.h b/gcc/selftest.h
index e86ce38..541bb71 100644
--- a/gcc/selftest.h
+++ b/gcc/selftest.h
@@ -153,6 +153,13 @@ for_each_line_table_case (void (*testcase) (const 
line_table_case &));
 
 extern char *read_file (const location &loc, const char *path);
 
+/* Read all of F_IN into memory, returning a 0-terminated buffer
+   that must be freed by the caller.  F_IN is *not* closed.
+   Fail (and abort) if there are any problems, with LOC as the reported
+   location of the failure, using DESC as a description of the file.  */
+
+extern char *read_file (const location &loc, FILE *infile, const char *desc);
+
 /* A helper function for writing tests that interact with the
garbage collector.  */
 
-- 
1.8.5.3

[PATCH 11/22] Add checkers/test-sources

[David Malcolm]

This patch adds a collection of test sources for use by the testsuites
of the various checker harnesses.

checkers/ChangeLog:
* test-sources/conditional-leak.c: New file.
* test-sources/cpychecker-demo.c: New file.
* test-sources/divide-by-zero.c: New file.
* test-sources/harmless.c: New file.
* test-sources/multiple-1.c: New file.
* test-sources/multiple-2.c: New file.
* test-sources/out-of-bounds.c: New file.
* test-sources/read-through-null.c: New file.
* test-sources/return-of-stack-address.c: New file.
* test-sources/unconditional-file-leak.c: New file.
---
 checkers/test-sources/conditional-leak.c|  17 
 checkers/test-sources/cpychecker-demo.c | 110 
 checkers/test-sources/divide-by-zero.c  |   4 +
 checkers/test-sources/harmless.c|   9 ++
 checkers/test-sources/multiple-1.c  |   6 ++
 checkers/test-sources/multiple-2.c  |   9 ++
 checkers/test-sources/out-of-bounds.c   |   6 ++
 checkers/test-sources/read-through-null.c   |   4 +
 checkers/test-sources/return-of-stack-address.c |   6 ++
 checkers/test-sources/unconditional-file-leak.c |  10 +++
 10 files changed, 181 insertions(+)
 create mode 100644 checkers/test-sources/conditional-leak.c
 create mode 100644 checkers/test-sources/cpychecker-demo.c
 create mode 100644 checkers/test-sources/divide-by-zero.c
 create mode 100644 checkers/test-sources/harmless.c
 create mode 100644 checkers/test-sources/multiple-1.c
 create mode 100644 checkers/test-sources/multiple-2.c
 create mode 100644 checkers/test-sources/out-of-bounds.c
 create mode 100644 checkers/test-sources/read-through-null.c
 create mode 100644 checkers/test-sources/return-of-stack-address.c
 create mode 100644 checkers/test-sources/unconditional-file-leak.c

diff --git a/checkers/test-sources/conditional-leak.c 
b/checkers/test-sources/conditional-leak.c
new file mode 100644
index 000..2ab46f5
--- /dev/null
+++ b/checkers/test-sources/conditional-leak.c
@@ -0,0 +1,17 @@
+#include 
+
+void test ()
+{
+  void *ptr_1;
+  void *ptr_2;
+
+  ptr_1 = malloc (64);
+  if (!ptr_1)
+return;
+  ptr_2 = malloc (64);
+  if (!ptr_2)
+return;
+
+  free (ptr_2);
+  free (ptr_1);
+}
diff --git a/checkers/test-sources/cpychecker-demo.c 
b/checkers/test-sources/cpychecker-demo.c
new file mode 100644
index 000..b379729
--- /dev/null
+++ b/checkers/test-sources/cpychecker-demo.c
@@ -0,0 +1,110 @@
+/*
+   Copyright 2011 David Malcolm 
+   Copyright 2011 Red Hat, Inc.
+
+   This is free software: you can redistribute it and/or modify it
+   under the terms of the GNU General Public License as published by
+   the Free Software Foundation, either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful, but
+   WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see
+   .
+*/
+
+/* Examples of mistakes made using the Python API */
+#include 
+
+extern uint16_t htons(uint16_t hostshort);
+
+PyObject *
+socket_htons(PyObject *self, PyObject *args)
+{
+unsigned long x1, x2;
+
+if (!PyArg_ParseTuple(args, "i:htons", &x1)) {
+return NULL;
+}
+x2 = (int)htons((short)x1);
+return PyInt_FromLong(x2);
+}
+
+PyObject *
+not_enough_varargs(PyObject *self, PyObject *args)
+{
+   if (!PyArg_ParseTuple(args, "i")) {
+   return NULL;
+   }
+   Py_RETURN_NONE;
+}
+
+PyObject *
+too_many_varargs(PyObject *self, PyObject *args)
+{
+int i, j;
+if (!PyArg_ParseTuple(args, "i", &i, &j)) {
+return NULL;
+}
+Py_RETURN_NONE;
+}
+
+PyObject *
+kwargs_example(PyObject *self, PyObject *args, PyObject *kwargs)
+{
+double x, y;
+char *keywords[] = {"x", "y"};
+
+if (!PyArg_ParseTupleAndKeywords(args, kwargs, "(ff):kwargs_example", 
keywords, &x, &y)) {
+return NULL;
+}
+Py_RETURN_NONE;
+}
+
+
+extern int convert_to_ssize(PyObject *, Py_ssize_t *);
+
+PyObject *
+buggy_converter(PyObject *self, PyObject *args)
+{
+int i;
+
+if (!PyArg_ParseTuple(args, "O&", convert_to_ssize, &i)) {
+return NULL;
+}
+
+Py_RETURN_NONE;
+}
+
+PyObject *
+make_a_list_of_random_ints_badly(PyObject *self,
+ PyObject *args)
+{
+PyObject *list, *item;
+long count, i;
+
+if (!PyArg_ParseTuple(args, "i", &count)) {
+ return NULL;
+}
+
+list = PyList_New(0);
+
+for (i = 0; i < count; i++) {
+item = PyLong_FromLong(random());
+PyList_Append(list, item);
+}
+
+return list;
+}
+
+/*
+  PEP-7
+Local variables:
+c-basic-offset: 4
+indent-tabs-mode: n

[PATCH 16/22] Add checkers/coverity.py

[David Malcolm]

This patch is an example of supporting a proprietary 3rd-party tool:
a harness for invoking the Coverity checker.

It uses the '--json-output-v2' option to cov-format-errors, and then uses
firehose.parsers.coverity.parse_json_v2 to parse the generated Coverity
JSON format, turning it into firehose JSON.

This isn't a great example of use of either the checker infrastructure, or
of Coverity.

As far as I can tell, Coverity is designed to be run on a
number of source files at once, performing a relatively cheap data-gathering
phase per-source-file, and then performing a more expensive LTO-style
analysis that can follow dataflow between source files, thus obtaining
much more accurate results that a purely one-file-at-a-time checker can.

In contrast, the checker machinery in this patch kit is designed to run
one file at a time.  The harness code in this patch attempts to "square
this circle", but it's not a good fit; it can detect problems that
are within one source file, but prevents the checker from finding
the more interesting problems that it's normally capable of.

checkers/ChangeLog:
* coverity.py: New file.
---
 checkers/coverity.py | 141 +++
 1 file changed, 141 insertions(+)
 create mode 100644 checkers/coverity.py

diff --git a/checkers/coverity.py b/checkers/coverity.py
new file mode 100644
index 000..533a6ae
--- /dev/null
+++ b/checkers/coverity.py
@@ -0,0 +1,141 @@
+#!/usr/bin/env python
+#   Copyright 2017 David Malcolm 
+#   Copyright 2017 Red Hat, Inc.
+#
+#   This is free software: you can redistribute it and/or modify it
+#   under the terms of the GNU General Public License as published by
+#   the Free Software Foundation, either version 3 of the License, or
+#   (at your option) any later version.
+#
+#   This program is distributed in the hope that it will be useful, but
+#   WITHOUT ANY WARRANTY; without even the implied warranty of
+#   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+#   General Public License for more details.
+#
+#   You should have received a copy of the GNU General Public License
+#   along with this program.  If not, see
+#   .
+
+#   Coverity is a trademark of Synopsys, Inc. in the U.S. and/or other
+#   countries.
+
+import os
+import sys
+import tempfile
+import unittest
+
+from gccinvocation import GccInvocation
+
+from checker import Checker, Context, CheckerTests, make_file, make_stats, \
+tool_main
+
+from firehose.model import Analysis, Generator, Metadata, Failure, \
+Location, File, Message, Issue, Trace
+from firehose.parsers.coverity import parse_json_v2
+
+os.environ['PATH'] = '/opt/coverity/bin:' + os.environ['PATH']
+
+class InvokeCoverity(Checker):
+"""
+Checker subclass that invokes Coverity
+"""
+name = 'coverity'
+
+def __init__(self, ctxt, verbose=False):
+Checker.__init__(self, ctxt)
+self.verbose = verbose
+
+def raw_invoke(self, gccinv, sourcefile):
+# tempfile.TemporaryDirectory is only available from Python 3.2 
onwards,
+# so handle tempdir cleanup "by hand"
+try:
+tempdir_name = tempfile.mkdtemp()
+
+json_name = os.path.join(tempdir_name, 'output.json')
+build_args = ['cov-build', '--dir', tempdir_name, 'gcc'] + 
gccinv.argv[1:]
+if self.verbose:
+print(build_args)
+build_result = self.run_subprocess(sourcefile, build_args)
+if self.verbose:
+print(build_result)
+
+analyze_args = ['cov-analyze', '--dir', tempdir_name,
+'--wait-for-license']
+analyze_result = self.run_subprocess(sourcefile, analyze_args)
+if self.verbose:
+print(analyze_result)
+
+format_args = ['cov-format-errors', '--dir', tempdir_name,
+   '--json-output-v2', json_name]
+format_result = self.run_subprocess(sourcefile, format_args)
+if self.verbose:
+print(format_result)
+
+# Parse the output, returning an Analysis instance
+analysis = parse_json_v2(json_name)
+if self.verbose:
+print(analysis)
+return analysis
+
+# FIXME: timing metadata?
+
+finally:
+pass # FIXME: cleanup tempdir
+
+class CoverityTests(CheckerTests):
+def make_tool(self):
+return self.make_tool_from_class(InvokeCoverity)
+
+def verify_basic_metadata(self, analysis, sourcefile):
+# Verify basic metadata:
+self.assert_metadata(analysis, 'coverity', sourcefile)
+
+def test_file_not_found(self):
+analysis = self.invoke('does-not-exist.c')
+self.assertEqual(len(analysis.results), 0)
+
+def test_timeout(self):
+sourcefile = 'test-sources/harmless.c'
+tool = self.make_tool()
+tool.timeout = 0
+g

[PATCH 04/22] Add firehose.h/cc

[David Malcolm]

"Firehose" is a serialization format for results from code
analysis tools:

  http://firehose.readthedocs.io/en/latest/index.html

(along with a Python module for working with the format).

This patch implements a set of C++ classes modeling the format,
with support for populating them from a JSON dump, so that we
can lossly serialize diagnostics and other static analysis results.

gcc/ChangeLog:
* Makefile.in (OBJS): Add firehose.o.
* firehose.cc: New file.
* firehose.h: New file.
* selftest-run-tests.c (selftest::run_tests): Call
selftest::firehose_cc_tests.
* selftest.h (selftest::firehose_cc_tests): New decl.

gcc/testsuite/ChangeLog:
* selftests/checker-output/test-clang-analyzer.json: New file.
* selftests/checker-output/test-cppcheck.json: New file.
* selftests/checker-output/test-failure.json: New file.
---
 gcc/Makefile.in|   1 +
 gcc/firehose.cc| 709 +
 gcc/firehose.h | 199 ++
 gcc/selftest-run-tests.c   |   1 +
 gcc/selftest.h |   1 +
 .../checker-output/test-clang-analyzer.json| 122 
 .../selftests/checker-output/test-cppcheck.json|  50 ++
 .../selftests/checker-output/test-failure.json |  38 ++
 8 files changed, 1121 insertions(+)
 create mode 100644 gcc/firehose.cc
 create mode 100644 gcc/firehose.h
 create mode 100644 
gcc/testsuite/selftests/checker-output/test-clang-analyzer.json
 create mode 100644 gcc/testsuite/selftests/checker-output/test-cppcheck.json
 create mode 100644 gcc/testsuite/selftests/checker-output/test-failure.json

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index 4f7fd0c..488f699 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -1280,6 +1280,7 @@ OBJS = \
expr.o \
fibonacci_heap.o \
final.o \
+   firehose.o \
fixed-value.o \
fold-const.o \
fold-const-call.o \
diff --git a/gcc/firehose.cc b/gcc/firehose.cc
new file mode 100644
index 000..b2aa167
--- /dev/null
+++ b/gcc/firehose.cc
@@ -0,0 +1,709 @@
+/* Serialization format for checker results.
+   Copyright (C) 2017 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify it under
+the terms of the GNU General Public License as published by the Free
+Software Foundation; either version 3, or (at your option) any later
+version.
+
+GCC is distributed in the hope that it will be useful, but WITHOUT ANY
+WARRANTY; without even the implied warranty of MERCHANTABILITY or
+FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3.  If not see
+.  */
+
+#include "config.h"
+#include "system.h"
+#include "coretypes.h"
+#include "firehose.h"
+#include "selftest.h"
+#include "selftest-input.h"
+
+namespace firehose {
+
+/* Attempt to parse JV as a json object containing "line" and "column"
+   attributes (a serialization of a firehose.model.Point python object).
+
+   If successful, write a location_t to OUT_VALUE, using GIVENPATH as the
+   filename, and return true.
+   Otherwise, write an error message to OUT_ERR (which must be freed by
+   the caller) and return false. */
+
+static bool
+get_location_from_point (const char *givenpath, const json::value *jv,
+location_t &out_value, char *&out_err)
+{
+  int line;
+  if (!jv->get_int_by_key ("line", line, out_err))
+return false;
+
+  int column;
+  if (!jv->get_int_by_key ("column", column, out_err))
+return false;
+
+  out_value
+   = linemap_position_for_file_line_and_column (line_table,
+   givenpath, line, column);
+  return true;
+}
+
+/* As above, but expect JV to be a json object containing a "start"
+   and "end" (a serialization of a firehose.model.Range python object).  */
+
+static bool
+get_location_from_range (const char *givenpath, const json::value *jv,
+location_t &out_value, char *&out_err)
+{
+  const json::value *jv_start;
+  if (!jv->get_value_by_key ("start", jv_start, out_err))
+return false;
+
+  location_t start;
+  if (!get_location_from_point (givenpath, jv_start,
+   start, out_err))
+return false;
+
+  const json::value *jv_end;
+  if (!jv->get_value_by_key ("end", jv_end, out_err))
+return false;
+  location_t end;
+  if (!get_location_from_point (givenpath, jv_end,
+   end, out_err))
+return false;
+
+  out_value = make_location (start, start, end);
+  return true;
+}
+
+/* Attempt to extract an attribute "location" from JV, where the value
+   ought to be a serialization of a firehose.model.Location python o

[PATCH 15/22] Add checkers/clang_analyzer.py

[David Malcolm]

This patch adds a harness for invoking clang's static analyzer:
  https://clang-analyzer.llvm.org/
returning the results in JSON format.

It runs scan-build, then uses firehose.parsers.clanganalyzer.parse_plist
to parse the generated .plist file, turning them into firehose JSON.

checkers/ChangeLog:
* clang_analyzer.py: New file.
---
 checkers/clang_analyzer.py | 145 +
 1 file changed, 145 insertions(+)
 create mode 100755 checkers/clang_analyzer.py

diff --git a/checkers/clang_analyzer.py b/checkers/clang_analyzer.py
new file mode 100755
index 000..ae41d93
--- /dev/null
+++ b/checkers/clang_analyzer.py
@@ -0,0 +1,145 @@
+#!/usr/bin/env python
+#   Copyright 2012, 2013, 2015, 2017 David Malcolm 
+#   Copyright 2012, 2013, 2015, 2017 Red Hat, Inc.
+#
+#   This is free software: you can redistribute it and/or modify it
+#   under the terms of the GNU General Public License as published by
+#   the Free Software Foundation, either version 3 of the License, or
+#   (at your option) any later version.
+#
+#   This program is distributed in the hope that it will be useful, but
+#   WITHOUT ANY WARRANTY; without even the implied warranty of
+#   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+#   General Public License for more details.
+#
+#   You should have received a copy of the GNU General Public License
+#   along with this program.  If not, see
+#   .
+
+import glob
+import os
+import sys
+import tempfile
+
+from gccinvocation import GccInvocation
+from firehose.model import Failure, Issue, Trace
+from firehose.parsers.clanganalyzer import parse_plist
+
+from checker import Checker, CheckerTests, make_file, make_stats, \
+tool_main
+
+class InvokeClangAnalyzer(Checker):
+"""
+Checker subclass that invokes the clang analyzer
+"""
+name = 'clang-analyzer'
+
+def raw_invoke(self, gccinv, sourcefile):
+self.resultdir = tempfile.mkdtemp()
+args = ['scan-build', '-v', '-plist',
+'--use-analyzer', '/usr/bin/clang', # rhbz 923834
+'-o', self.resultdir,
+'gcc'] + gccinv.argv[1:]
+return self._run_subprocess(sourcefile, args)
+
+def handle_output(self, result):
+if result.returncode:
+analysis = self._make_failed_analysis(result.sourcefile, 
result.timer,
+  msgtext='Bad exit code 
running %s' % self.name,
+  failureid='bad-exit-code')
+self.set_custom_fields(result, analysis)
+return analysis
+
+# Given e.g. resultdir='/tmp/tmpQW2l2B', the plist files
+# are an extra level deep e.g.:
+#  '/tmp/tmpQW2l2B/2013-01-22-1/report-MlwJri.plist'
+self.log(self.resultdir)
+for plistpath in glob.glob(os.path.join(self.resultdir,
+'*/*.plist')):
+analysis = parse_plist(plistpath,
+   file_=make_file(result.sourcefile),
+   stats=make_stats(result.timer))
+self.set_custom_fields(result, analysis)
+analysis.set_custom_field('plistpath', plistpath)
+return analysis # could there be more than one?
+
+# Not found?
+analysis = self._make_failed_analysis(
+result.sourcefile, result.timer,
+msgtext='Unable to locate plist file',
+failureid='plist-not-found')
+self.set_custom_fields(result, analysis)
+return analysis
+
+def set_custom_fields(self, result, analysis):
+analysis.set_custom_field('scan-build-invocation',
+  ' '.join(result.argv))
+result.set_custom_fields(analysis)
+
+class ClangAnalyzerTests(CheckerTests):
+def make_tool(self):
+return self.make_tool_from_class(InvokeClangAnalyzer)
+
+def verify_basic_metadata(self, analysis, sourcefile):
+# Verify basic metadata:
+self.assert_metadata(analysis, 'clang-analyzer', sourcefile)
+self.assert_has_custom_field(analysis, 'scan-build-invocation')
+self.assert_has_custom_field(analysis, 'stdout')
+self.assert_has_custom_field(analysis, 'stderr')
+
+def test_file_not_found(self):
+analysis = self.invoke('does-not-exist.c')
+#print(analysis)
+self.assertEqual(len(analysis.results), 1)
+self.assertIsInstance(analysis.results[0], Failure)
+self.assertEqual(analysis.results[0].failureid, 'bad-exit-code')
+
+def test_timeout(self):
+sourcefile = 'test-sources/harmless.c'
+tool = self.make_tool()
+tool.timeout = 0
+gccinv = GccInvocation(['gcc', sourcefile])
+analysis = tool.checked_invoke(gccinv, sourcefile)
+self.assert_metadata(analysis, 'clang-analyzer', sourcefile)
+self.as

[PATCH 06/22] Makefile.in: hack in -lpthread

[David Malcolm]

The checker.cc patch later in the kit can optionally make use of pthread
if available.

Doing it properly would involve some configure checks; this patch simply
hacks in -lpthread into LIB unconditionally for now.

gcc/ChangeLog:
* Makefile.in (LIB): Hack in -lpthread.
---
 gcc/Makefile.in | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index 488f699..9ceb3f3 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -1048,10 +1048,11 @@ LIBDEPS= libcommon.a $(CPPLIB) $(LIBIBERTY) 
$(LIBINTL_DEP) $(LIBICONV_DEP) \
 # even if we are cross-building GCC.
 BUILD_LIBDEPS= $(BUILD_LIBIBERTY)
 
+# FIXME: add some configury for pthread
 # How to link with both our special library facilities
 # and the system's installed libraries.
 LIBS = @LIBS@ libcommon.a $(CPPLIB) $(LIBINTL) $(LIBICONV) $(LIBBACKTRACE) \
-   $(LIBIBERTY) $(LIBDECNUMBER) $(HOST_LIBS)
+   $(LIBIBERTY) $(LIBDECNUMBER) $(HOST_LIBS) -lpthread
 BACKENDLIBS = $(ISLLIBS) $(GMPLIBS) $(PLUGINLIBS) $(HOST_LIBS) \
$(ZLIB)
 # Any system libraries needed just for GNAT.
-- 
1.8.5.3

[PATCH 14/22] Add checkers/always_fails.py

[David Malcolm]

This patch adds a "checker" that always fails, for ensuring that
we can handle failed runs of 3rd-party tools.

checkers/ChangeLog:
* always_fails.py: New file.
---
 checkers/always_fails.py | 57 
 1 file changed, 57 insertions(+)
 create mode 100755 checkers/always_fails.py

diff --git a/checkers/always_fails.py b/checkers/always_fails.py
new file mode 100755
index 000..35fd4ac
--- /dev/null
+++ b/checkers/always_fails.py
@@ -0,0 +1,57 @@
+#!/usr/bin/env python
+#   Copyright 2012, 2013, 2015, 2017 David Malcolm 
+#   Copyright 2012, 2013, 2015, 2017 Red Hat, Inc.
+#
+#   This is free software: you can redistribute it and/or modify it
+#   under the terms of the GNU General Public License as published by
+#   the Free Software Foundation, either version 3 of the License, or
+#   (at your option) any later version.
+#
+#   This program is distributed in the hope that it will be useful, but
+#   WITHOUT ANY WARRANTY; without even the implied warranty of
+#   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+#   General Public License for more details.
+#
+#   You should have received a copy of the GNU General Public License
+#   along with this program.  If not, see
+#   .
+
+import sys
+import unittest
+
+from firehose.model import Failure
+
+from checker import Checker, CheckerTests, tool_main
+
+class AlwaysFails(Checker):
+"""
+Checker subclass that always fails
+"""
+name = 'always-fails'
+
+def raw_invoke(self, gccinv, sourcefile):
+args = ['/this/executable/does/not/exist', sourcefile]
+return self._run_subprocess(sourcefile, args)
+
+class AlwaysFailsTests(CheckerTests):
+def make_tool(self):
+tool_class = AlwaysFails
+ctxt = self.make_ctxt(tool_class.name, capture_exceptions=True)
+return tool_class(ctxt)
+
+def verify_basic_metadata(self, analysis, sourcefile):
+# Verify basic metadata:
+self.assert_metadata(analysis, 'always-fails', sourcefile)
+
+def test_harmless_file(self):
+analysis = self.invoke('test-sources/harmless.c')
+self.assertEqual(len(analysis.results), 1)
+r0 = analysis.results[0]
+self.assertIsInstance(r0, Failure)
+self.assertEqual(r0.failureid, 'exception')
+self.assertEqual(r0.location.file.givenpath,
+ 'test-sources/harmless.c')
+self.assertNotEqual(r0.message.text, None)
+
+if __name__ == '__main__':
+sys.exit(tool_main(sys.argv, AlwaysFails))
-- 
1.8.5.3

[PATCH 12/22] Add -Wrun-analyzers= to common.opt, toplev.c, and invoke.texi

[David Malcolm]

This patch introduces -Wrun-analyzers= and wires it up to the checkers.cc
code, via toplev.c

As noted earlier, I'm not a great fan of the monolithic option syntax
here; maybe there should be some kind of option that can be supplied
multiple times for adding individual checkers.

gcc/ChangeLog:
* common.opt (Wrun-analyzers=): New option.
* doc/invoke.texi (Warning Options): Add -Wrun-analyzers=.
(-Wrun-analyzers=): New option.
* toplev.c: Include "checkers.h".
(compile_file): Call checkers_finish.
(do_compile): Call checkers_start.
---
 gcc/common.opt  | 4 
 gcc/doc/invoke.texi | 8 +++-
 gcc/toplev.c| 9 +
 3 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/gcc/common.opt b/gcc/common.opt
index 1cb1c83..5a7b47d 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -669,6 +669,10 @@ Wreturn-local-addr
 Common Var(warn_return_local_addr) Init(1) Warning
 Warn about returning a pointer/reference to a local or temporary variable.
 
+Wrun-analyzers=
+Common Joined Var(warn_run_analyzers) Warning
+Run 3rd-party analyzer tools based on the supplied JSON file.
+
 Wshadow
 Common Var(warn_shadow) Warning
 Warn when one variable shadows another.  Same as -Wshadow=global.
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 5ae9dc4..6756c07 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -303,7 +303,7 @@ Objective-C and Objective-C++ Dialects}.
 -Wplacement-new  -Wplacement-new=@var{n} @gol
 -Wpointer-arith  -Wpointer-compare  -Wno-pointer-to-int-cast @gol
 -Wno-pragmas  -Wredundant-decls  -Wrestrict  -Wno-return-local-addr @gol
--Wreturn-type  -Wsequence-point  -Wshadow  -Wno-shadow-ivar @gol
+-Wreturn-type  -Wrun-analyzers=  -Wsequence-point  -Wshadow  -Wno-shadow-ivar 
@gol
 -Wshadow=global,  -Wshadow=local,  -Wshadow=compatible-local @gol
 -Wshift-overflow  -Wshift-overflow=@var{n} @gol
 -Wshift-count-negative  -Wshift-count-overflow  -Wshift-negative-value @gol
@@ -4638,6 +4638,12 @@ exceptions are @code{main} and functions defined in 
system headers.
 
 This warning is enabled by @option{-Wall}.
 
+@item -Wrun-analyzers=@var{path-to-json}
+@opindex Wrun-analyzers
+Run 3rd-party analyzer tools based on the supplied JSON file.
+
+FIXME: document the format here
+
 @item -Wshift-count-negative
 @opindex Wshift-count-negative
 @opindex Wno-shift-count-negative
diff --git a/gcc/toplev.c b/gcc/toplev.c
index b28f184..4f871e0 100644
--- a/gcc/toplev.c
+++ b/gcc/toplev.c
@@ -80,6 +80,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "hsa-common.h"
 #include "edit-context.h"
 #include "tree-pass.h"
+#include "checkers.h"
 
 #if defined(DBX_DEBUGGING_INFO) || defined(XCOFF_DEBUGGING_INFO)
 #include "dbxout.h"
@@ -501,6 +502,11 @@ compile_file (void)
   if (lang_hooks.decls.post_compilation_parsing_cleanups)
 lang_hooks.decls.post_compilation_parsing_cleanups ();
 
+  /* If any 3rd-party analyzers are being run, wait for them to
+ finish, and handle the results.  */
+  if (warn_run_analyzers)
+checkers_finish ();
+
   if (seen_error ())
 return;
 
@@ -1983,6 +1989,9 @@ do_compile ()
 {
   int i;
 
+  if (warn_run_analyzers)
+   checkers_start (warn_run_analyzers);
+
   timevar_start (TV_PHASE_SETUP);
 
   /* This must be run always, because it is needed to compute the FP
-- 
1.8.5.3

[PATCH 21/22] Add checkers/Makefile

[David Malcolm]

This patch adds a rather simplistic Makefile to "checkers", purely for
exercising the test suites of the various harness.

checkers/ChangeLog:
* Makefile: New file.
---
 checkers/Makefile | 23 +++
 1 file changed, 23 insertions(+)
 create mode 100644 checkers/Makefile

diff --git a/checkers/Makefile b/checkers/Makefile
new file mode 100644
index 000..f69dccf
--- /dev/null
+++ b/checkers/Makefile
@@ -0,0 +1,23 @@
+all: check-all
+
+check-all: check-all-python2 check-all-python3
+
+check-all-python2:
+   python2 checker.py
+   python2 clang_analyzer.py unittest
+   python2 cppcheck.py unittest
+   python2 flawfinder.py unittest
+   python2 splint.py unittest
+   python2 always_fails.py unittest
+   python2 ianal.py unittest
+   python2 coverity.py unittest
+
+check-all-python3:
+   python3 checker.py
+   python3 clang_analyzer.py unittest
+   python3 cppcheck.py unittest
+   python3 flawfinder.py unittest
+   python3 splint.py unittest
+   python3 always_fails.py unittest
+   python3 ianal.py unittest
+   python3 coverity.py unittest
-- 
1.8.5.3

[PATCH 18/22] Add checkers/flawfinder.py

[David Malcolm]

This patch adds a harness for invoking flawfinder:
  https://www.dwheeler.com/flawfinder/
returning the results in JSON format.

It runs "flawfinder", then uses firehose.parsers.flawfinder.parse_file
to parse the stdout, turning it into firehose JSON.

checkers/ChangeLog:
* flawfinder.py: New file.
---
 checkers/flawfinder.py | 124 +
 1 file changed, 124 insertions(+)
 create mode 100755 checkers/flawfinder.py

diff --git a/checkers/flawfinder.py b/checkers/flawfinder.py
new file mode 100755
index 000..475a513
--- /dev/null
+++ b/checkers/flawfinder.py
@@ -0,0 +1,124 @@
+#!/usr/bin/env python
+#   Copyright 2012, 2013, 2015, 2017 David Malcolm 
+#   Copyright 2012, 2013, 2015, 2017 Red Hat, Inc.
+#
+#   This is free software: you can redistribute it and/or modify it
+#   under the terms of the GNU General Public License as published by
+#   the Free Software Foundation, either version 3 of the License, or
+#   (at your option) any later version.
+#
+#   This program is distributed in the hope that it will be useful, but
+#   WITHOUT ANY WARRANTY; without even the implied warranty of
+#   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+#   General Public License for more details.
+#
+#   You should have received a copy of the GNU General Public License
+#   along with this program.  If not, see
+#   .
+
+import sys
+import tempfile
+
+from gccinvocation import GccInvocation
+
+from checker import Checker, CheckerTests, make_file, make_stats, \
+tool_main
+
+from firehose.model import Failure, Issue
+from firehose.parsers.flawfinder import parse_file
+
+class InvokeFlawfinder(Checker):
+"""
+Checker subclass that invokes "flawfinder"
+"""
+name = 'flawfinder'
+
+def raw_invoke(self, gccinv, sourcefile):
+args = ['flawfinder', sourcefile] # FIXME
+return self._run_subprocess(sourcefile, args)
+
+def handle_output(self, result):
+if result.returncode:
+analysis = self._make_failed_analysis(result.sourcefile, 
result.timer,
+  msgtext='Bad exit code 
running %s' % self.name,
+  failureid='bad-exit-code')
+self.set_custom_fields(result, analysis)
+return analysis
+
+if 0:
+print('result.err: %r' % result.err)
+print('result.out: %r' % result.out)
+
+# (there doesn't seem to be a way to have flawfinder directly
+# save its output to a given location)
+
+with tempfile.NamedTemporaryFile() as outfile:
+outfile.write(result.out)
+outfile.flush()
+
+with open(outfile.name) as infile:
+# Parse stderr into firehose XML format and save:
+analysis = parse_file(infile)
+analysis.metadata.file_ = make_file(result.sourcefile)
+analysis.metadata.stats = make_stats(result.timer)
+self.set_custom_fields(result, analysis)
+
+return analysis
+
+def set_custom_fields(self, result, analysis):
+analysis.set_custom_field('flawfinder-invocation',
+  ' '.join(result.argv))
+result.set_custom_fields(analysis)
+
+class FlawfinderTests(CheckerTests):
+def make_tool(self):
+return self.make_tool_from_class(InvokeFlawfinder)
+
+def verify_basic_metadata(self, analysis, sourcefile):
+# Verify basic metadata:
+self.assert_metadata(analysis, 'flawfinder', sourcefile)
+self.assert_has_custom_field(analysis, 'flawfinder-invocation')
+self.assert_has_custom_field(analysis, 'stdout')
+self.assert_has_custom_field(analysis, 'stderr')
+
+def test_file_not_found(self):
+analysis = self.invoke('does-not-exist.c')
+#print(analysis)
+self.assertEqual(len(analysis.results), 0)
+
+def test_timeout(self):
+sourcefile = 'test-sources/harmless.c'
+tool = self.make_tool()
+tool.timeout = 0
+gccinv = GccInvocation(['gcc', sourcefile])
+analysis = tool.checked_invoke(gccinv, sourcefile)
+self.assert_metadata(analysis, 'flawfinder', sourcefile)
+self.assertEqual(len(analysis.results), 1)
+r0 = analysis.results[0]
+self.assertIsInstance(r0, Failure)
+self.assertEqual(r0.failureid, 'timeout')
+self.assert_has_custom_field(analysis, 'timeout')
+self.assert_has_custom_field(analysis, 'command-line')
+
+def test_harmless_file(self):
+analysis = self.invoke('test-sources/harmless.c')
+self.assertEqual(len(analysis.results), 0)
+
+def test_use_of_random(self):
+analysis = self.invoke('test-sources/cpychecker-demo.c')
+self.assertEqual(len(analysis.results), 1)
+r0 = analysis.results[0]
+self.assertIsInstance(r

[PATCH 13/22] Add checkers/checker.py

[David Malcolm]

This patch:
* creates a new "checkers" top-level directory to hold
  harnesses for 3rd-party code-checking tools.
* adds a "checker.py" Python module for use when implementing
  such harnesses

3rd-party code-checking tools are expected to be passed
command-line arguments by the frontend, and to return a JSON
result on stdout; the job of each harness is to coerce the
output from the tool into the expected JSON output format.

The JSON format to be used is the "Firehose" serialization
format:
  http://firehose.readthedocs.io/en/latest/index.html

checkers/ChangeLog:
* ChangeLog: New file.
* checker.py: New file.
---
 checkers/ChangeLog  |   9 ++
 checkers/checker.py | 367 
 2 files changed, 376 insertions(+)
 create mode 100644 checkers/ChangeLog
 create mode 100755 checkers/checker.py

diff --git a/checkers/ChangeLog b/checkers/ChangeLog
new file mode 100644
index 000..9189883
--- /dev/null
+++ b/checkers/ChangeLog
@@ -0,0 +1,9 @@
+2017-08-03  David Malcolm  
+
+   * ChangeLog: New ChangeLog file.
+
+Copyright (C) 2017 Free Software Foundation, Inc.
+
+Copying and distribution of this file, with or without modification,
+are permitted in any medium without royalty provided the copyright
+notice and this notice are preserved.
diff --git a/checkers/checker.py b/checkers/checker.py
new file mode 100755
index 000..262bd72
--- /dev/null
+++ b/checkers/checker.py
@@ -0,0 +1,367 @@
+#!/usr/bin/env python
+#   Copyright 2012, 2013, 2015, 2017 David Malcolm 
+#   Copyright 2012, 2013, 2015, 2017 Red Hat, Inc.
+#
+#   This is free software: you can redistribute it and/or modify it
+#   under the terms of the GNU General Public License as published by
+#   the Free Software Foundation, either version 3 of the License, or
+#   (at your option) any later version.
+#
+#   This program is distributed in the hope that it will be useful, but
+#   WITHOUT ANY WARRANTY; without even the implied warranty of
+#   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+#   General Public License for more details.
+#
+#   You should have received a copy of the GNU General Public License
+#   along with this program.  If not, see
+#   .
+
+"""
+A "checker" is an executable which takes GCC-style command-line
+arguments and writes a Firehose JSON file to stdout.
+"""
+
+import json
+import logging
+import os
+import re
+import sys
+import tempfile
+import time
+import traceback
+import unittest
+
+if sys.version_info[0] < 3:
+# http://pypi.python.org/pypi/subprocess32
+# so that we can use timeouts
+from subprocess32 import Popen, PIPE, STDOUT, TimeoutExpired
+else:
+from subprocess import Popen, PIPE, STDOUT, TimeoutExpired
+
+from firehose.model import Analysis, Generator, Metadata, Failure, \
+Location, File, Message, Issue, Trace
+
+from gccinvocation import GccInvocation
+
+def make_file(givenpath):
+from firehose.model import File
+return File(givenpath=givenpath,
+abspath=None,
+hash_=None)
+
+def make_stats(timer):
+from firehose.model import Stats
+return Stats(wallclocktime=timer.get_elapsed_time())
+
+class Timer:
+"""
+Simple measurement of wallclock time taken
+"""
+def __init__(self):
+self.starttime = time.time()
+
+def get_elapsed_time(self):
+"""Get elapsed time in seconds as a float"""
+curtime = time.time()
+return curtime - self.starttime
+
+def elapsed_time_as_str(self):
+"""Get elapsed time as a string (with units)"""
+elapsed = self.get_elapsed_time()
+result = '%0.3f seconds' % elapsed
+if elapsed > 120:
+result += ' (%i minutes)' % int(elapsed / 60)
+return result
+
+class Context:
+def __init__(self, name, enable_logging=False, capture_exceptions=True):
+self.name = name
+self.enable_logging = enable_logging
+self.capture_exceptions = capture_exceptions
+if self.enable_logging:
+format_ = '%(asctime)s ' + name + ': %(message)s'
+logging.basicConfig(format=format_,
+level=logging.INFO,
+stream=sys.stderr)
+self.log('logging initialized')
+
+self.stdout = sys.stdout
+self.stderr = sys.stderr
+self.returncode = None
+
+def log(self, msg):
+if self.enable_logging:
+logging.info(msg)
+
+def write_streams(self, toolname, out, err):
+for line in out.splitlines():
+self.log('stdout from %r: %s\n' % (toolname, line))
+for line in err.splitlines():
+self.log('stderr from %r: %s\n' % (toolname, line))
+
+class SubprocessResult:
+"""
+A bundle of information relating to a subprocess invocation.
+"""
+def __init__(self, sourcefile, argv, returncode, out, err, timer):
+self.sourcef

[PATCH 20/22] Add checkers/splint.py

[David Malcolm]

This patch adds a harness for invoking splint:
  http://splint.org/
returning the results in JSON format.

It runs "splint -csv TEMPFILE +csvoverwrite -strict", then uses
  firehose.parsers.splint.parse_splint_csv
and
  firehose.parsers.splint.parse_splint_stderr
to parse the csv and the stderr, turning them into firehose JSON
(stderr is used to get at version information).

checkers/ChangeLog:
* splint.py: New file.
---
 checkers/splint.py | 77 ++
 1 file changed, 77 insertions(+)
 create mode 100755 checkers/splint.py

diff --git a/checkers/splint.py b/checkers/splint.py
new file mode 100755
index 000..e8f79f2
--- /dev/null
+++ b/checkers/splint.py
@@ -0,0 +1,77 @@
+#!/usr/bin/env python
+#   Copyright 2017 David Malcolm 
+#   Copyright 2017 Red Hat, Inc.
+#
+#   This is free software: you can redistribute it and/or modify it
+#   under the terms of the GNU General Public License as published by
+#   the Free Software Foundation, either version 3 of the License, or
+#   (at your option) any later version.
+#
+#   This program is distributed in the hope that it will be useful, but
+#   WITHOUT ANY WARRANTY; without even the implied warranty of
+#   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+#   General Public License for more details.
+#
+#   You should have received a copy of the GNU General Public License
+#   along with this program.  If not, see
+#   .
+
+import sys
+import tempfile
+
+from firehose.model import File, Issue
+from firehose.parsers.splint import parse_splint_csv, parse_splint_stderr
+
+from checker import Checker, CheckerTests, tool_main
+
+class InvokeSplint(Checker):
+"""
+Checker subclass that invokes "splint -strict"
+"""
+name = 'splint'
+
+def __init__(self, ctxt):
+Checker.__init__(self, ctxt)
+self.tempfile = None
+
+def __del__(self):
+del self.tempfile
+
+def raw_invoke(self, gccinv, sourcefile):
+self.tempfile = tempfile.NamedTemporaryFile()
+args = ['splint', '-csv', self.tempfile.name, '+csvoverwrite', 
'-strict', sourcefile]
+# FIXME: why is overwrite needed?
+return self._run_subprocess(sourcefile, args)
+
+def handle_output(self, result):
+analysis = parse_splint_csv(self.tempfile.name)
+analysis.metadata.file_ = File(result.sourcefile, None)
+analysis.metadata.version = parse_splint_stderr(result.err)
+self.set_custom_fields(result, analysis)
+return analysis
+
+def set_custom_fields(self, result, analysis):
+analysis.set_custom_field('splint-invocation',
+  ' '.join(result.argv))
+result.set_custom_fields(analysis)
+
+class SplintTests(CheckerTests):
+def make_tool(self):
+return self.make_tool_from_class(InvokeSplint)
+
+def verify_basic_metadata(self, analysis, sourcefile):
+# Verify basic metadata:
+self.assert_metadata(analysis, 'splint', sourcefile)
+self.assert_has_custom_field(analysis, 'splint-invocation')
+self.assert_has_custom_field(analysis, 'stdout')
+self.assert_has_custom_field(analysis, 'stderr')
+
+def test_unconditional_leak(self):
+analysis = self.invoke('test-sources/unconditional-file-leak.c')
+self.assertEqual(len(analysis.results), 8)
+r0 = analysis.results[0]
+self.assertIsInstance(r0, Issue)
+self.assertEqual(r0.testid, 'internalglobs')
+
+if __name__ == '__main__':
+sys.exit(tool_main(sys.argv, InvokeSplint))
-- 
1.8.5.3

[PATCH 05/22] diagnostic.c/h: add support for external tools

[David Malcolm]

This patch adds fields "external_tool" and "external_test_id"
to diagnostic_info, allowing for diagnostics to be marked as
coming from a 3rd-party tool.

Instead of printing the pertinent warning flag e.g.:

  foo.c:10:1: something is wrong [-Wpointer-arith]

the tool "ID" and (optionally) test ID is printed e.g.:

  foo.c:10:1: something is wrong [cppcheck:memleak]

gcc/ChangeLog:
* diagnostic-show-locus.c: Include "selftest-diagnostic.h".
(class selftest::test_diagnostic_context): Move to
selftest-diagnostic.h.
* diagnostic.c: Include "selftest-diagnostic.h".
(diagnostic_info::diagnostic_info): New ctor.
(print_option_information): Handle external_tool and
external_test_id fields of diagnostic_info.
(diagnostic_report_diagnostic): Assert that diagnostic->kind is
not DK_UNSPECIFIED.
(selftest::dummy_option_name_cb): New function.
(selftest::assert_option_information): New function.
(selftest::test_print_option_information): New function.
(selftest::diagnostic_c_tests): Call
selftest::test_print_option_information.
* diagnostic.h (struct diagnostic_info): Add default ctor,
along with new fields "external_tool" and "external_test_id".
* selftest-diagnostic.h: New file.
---
 gcc/diagnostic-show-locus.c | 29 +---
 gcc/diagnostic.c| 85 ++---
 gcc/diagnostic.h|  5 +++
 gcc/selftest-diagnostic.h   | 62 +
 4 files changed, 149 insertions(+), 32 deletions(-)
 create mode 100644 gcc/selftest-diagnostic.h

diff --git a/gcc/diagnostic-show-locus.c b/gcc/diagnostic-show-locus.c
index b0e72e7..08b2e56 100644
--- a/gcc/diagnostic-show-locus.c
+++ b/gcc/diagnostic-show-locus.c
@@ -29,6 +29,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "diagnostic-color.h"
 #include "gcc-rich-location.h"
 #include "selftest.h"
+#include "selftest-diagnostic.h"
 
 #ifdef HAVE_TERMIOS_H
 # include 
@@ -1988,34 +1989,6 @@ namespace selftest {
 
 /* Selftests for diagnostic_show_locus.  */
 
-/* Convenience subclass of diagnostic_context for testing
-   diagnostic_show_locus.  */
-
-class test_diagnostic_context : public diagnostic_context
-{
- public:
-  test_diagnostic_context ()
-  {
-diagnostic_initialize (this, 0);
-show_caret = true;
-show_column = true;
-start_span = start_span_cb;
-  }
-  ~test_diagnostic_context ()
-  {
-diagnostic_finish (this);
-  }
-
-  /* Implementation of diagnostic_start_span_fn, hiding the
- real filename (to avoid printing the names of tempfiles).  */
-  static void
-  start_span_cb (diagnostic_context *context, expanded_location exploc)
-  {
-exploc.file = "FILENAME";
-default_diagnostic_start_span_fn (context, exploc);
-  }
-};
-
 /* Verify that diagnostic_show_locus works sanely on UNKNOWN_LOCATION.  */
 
 static void
diff --git a/gcc/diagnostic.c b/gcc/diagnostic.c
index bbf5f5c..570f8c2 100644
--- a/gcc/diagnostic.c
+++ b/gcc/diagnostic.c
@@ -33,6 +33,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "diagnostic-color.h"
 #include "edit-context.h"
 #include "selftest.h"
+#include "selftest-diagnostic.h"
 
 #ifdef HAVE_TERMIOS_H
 # include 
@@ -67,7 +68,17 @@ const char *progname;
 /* A diagnostic_context surrogate for stderr.  */
 static diagnostic_context global_diagnostic_context;
 diagnostic_context *global_dc = &global_diagnostic_context;
+
 
+
+/* diagnostic_info's ctor.  */
+
+diagnostic_info::diagnostic_info ()
+: message (), richloc (NULL), x_data (NULL), kind (DK_UNSPECIFIED),
+  option_index (0), external_tool (NULL), external_test_id (NULL)
+{
+}
+
 /* Return a malloc'd string containing MSG formatted a la printf.  The
caller is responsible for freeing the memory.  */
 char *
@@ -843,6 +854,28 @@ print_option_information (diagnostic_context *context,
  const diagnostic_info *diagnostic,
  diagnostic_t orig_diag_kind)
 {
+  pretty_printer *pp = context->printer;
+  const char *cs = colorize_start (pp_show_color (pp),
+  diagnostic_kind_color[diagnostic->kind]);
+  const char *ce = colorize_stop (pp_show_color (pp));
+
+  if (diagnostic->external_tool)
+{
+  pp_string (pp, " [");
+  pp_string (pp, cs);
+  pp_string (pp, diagnostic->external_tool);
+  pp_string (pp, ce);
+  if (diagnostic->external_test_id)
+   {
+ pp_character (pp, ':');
+ pp_string (pp, cs);
+ pp_string (pp, diagnostic->external_test_id);
+ pp_string (pp, ce);
+   }
+  pp_character (pp, ']');
+  return;
+}
+
   char *option_text;
 
   option_text = context->option_name (context, diagnostic->option_index,
@@ -850,12 +883,10 @@ print_option_information (diagnostic_context *context,
 
   if (option_text)
 {
-  pretty_printer *pp = context->printer;
   pp_

Re: [PATCH] Simplify pow with constant

[Joseph Myers]

On Fri, 4 Aug 2017, Richard Biener wrote:

> >> Do this only for fast-math as accuracy is reduced.  This is much faster
> >> since pow is more complex than exp - with a current GLIBC the speedup
> >> is more than 7 times for this transformation.
> >
> > Is it bound to be so on future glibc revisions and non-glibc platforms?
> 
> And how is accuracy affected?  I think the transform is only reasonable

For pow to be accurate when the result has large (positive or negative) 
exponent, it needs to compute the log and intermediate multiplication to a 
precision around (number of mantissa bits + number of exponent bits).  
This is inevitably slower than when you omit the extra intermediate 
precision (and if you omit that precision, the error can be around MAX_EXP 
ulps).

-- 
Joseph S. Myers
jos...@codesourcery.com

Re: [PATCH 0/2] add unique_ptr class

[Martin Sebor]

On 07/31/2017 05:46 PM, tbsaunde+...@tbsaunde.org wrote:

From: Trevor Saunders 

Hi,

I've been saying I'd do this for a long time, but I'm finally getting to
importing the C++98 compatable unique_ptr class Pedro wrote for gdb a while
back.  I believe the gtl namespace also comes from Pedro, but GNU template
library seems as reasonable as any other name I can come up with.  I'm not sure
at the moment what outside of gcc may want to use this, but putting it include/
at least allows us to use it in libcpp which may be useful.  I didn't include
too much usage in this series, but I believe other people have wanted this too,
so I'm reasonably confident it will get a fair amount of usage.

patches individually bootstrapped + regtested on ppc64-linux-gnu, ok?


FWIW, I'm a big fan of RAII and I like the idea being able to
rely on a smart pointer in GCC quite a bit.  Although I haven't
reviewed the C++ 03 implementation in any detail I like how the
new API is just transitional until GCC switches from C++ 98 to
C++ 11 when it will be possible to drop it, presumably with no
changes to client code.  (If there's any risk that clients might
come to depend on the C++ 03 "features" or limitations in the
meantime and thus jeopardize this goal I would only suggest to
put some effort into making this harder.)

Other than that, thank you for this nice enhancement!

Martin

Re: [PATCH 00/22] RFC: integrated 3rd-party static analysis support

[Eric Gallager]

On 8/4/17, David Malcolm  wrote:
> This patch kit clearly isn't ready yet as-is (see e.g. the
> "known unknowns" below), but I'm posting it now in the hope of
> getting early feedback.
>
> Summary
> ===
>
> This patch kit provides an easy way to make integrate 3rd-party static
> analysis tools into gcc, and have them:
> (a) report through gcc's diagnostic subsystem, and
> (b) "watermark" the generated binaries with queryable data on what checkers
> were run, and what the results were.
>
> Here's an example showing gcc running a bank of 3rd-party checkers on this
> source file:
>
>   #include 
>
>   void test ()
>   {
> void *ptr_1;
> void *ptr_2;
>
> ptr_1 = malloc (64);
> if (!ptr_1)
>   return;
> ptr_2 = malloc (64);
> if (!ptr_2)
>   return;
>
> free (ptr_2);
> free (ptr_1);
>   }
>
> via a simple command-line:
>
>   $ ./xgcc -B. -c conditional-leak.c -Wrun-analyzers=policy.json
>   conditional-leak.c:13:5: error: Potential leak of memory pointed to by
> 'ptr_1' [clang-analyzer:Memory leak]
>return;
>^
>   conditional-leak.c:8:11: note: state 1 of 4: Memory is allocated
>  ptr_1 = malloc (64);
>  ^
>   conditional-leak.c:9:7: note: state 2 of 4: Assuming 'ptr_1' is non-null
>  if (!ptr_1)
>  ^
>   conditional-leak.c:12:7: note: state 3 of 4: Assuming 'ptr_2' is null
>  if (!ptr_2)
>  ^
>   conditional-leak.c:13:5: note: state 4 of 4: Potential leak of memory
> pointed to by 'ptr_1'
>return;
>^
>   conditional-leak.c:13:0: error: Memory leak: ptr_1 [cppcheck:memleak]
>return;
>
> Of the checkers, clang's static analyzer and cppcheck both identify the
> memory leak; the former also identifies the control flow (the other
> checkers didn't report anything).
>
> The idea is to provide a mechanism to make it easy for developers and
> projects to impose policy on what checkers should be run, and to gate
> the build if certain tests fail.
>
> In this case, the results are treated as hard errors and block the build,
> but policy could allow them to be warnings.
>
> Extensive metadata is captured about what checkers were run, and what
> they emitted, using the "Firehose" interchange format:
>
>   http://firehose.readthedocs.io/en/latest/index.html
>
> In the case where this doesn't block the build, this can be queried via a
>   contrib/get-static-analysis.py
> script, so e.g. you can verify that a setuid binary was indeed compiled
> using all the checkers that you expect it to be.
>
> This can also be used to embed data about the code into the watermark.
> For example, checkers/ianal.py embeds information about "Copyright"
> lines in the source code into the generated binaries, from where it
> can be queried (this example is intended as a proof-of-concept rather
> than as a real license-tracking solution...)
>
>
> Statement of the problem
> 
>
> Static analysis is IMHO done too late, if at all: static analysis tools are
> run
> as an optional extra, "on the side", rather than in developers' normal
> workflow, with some kind of "override the compiler and do extra work" hook,
> which may preclude running more than one analyzer at once.  Analysis
> results
> are reviewed (if at all) in some kind of on-the-side tool, rather than when
> the
> code is being edited, or patches being prepared.
>
> It would be better to have an easy way for developers to run analyzer(s)
> as they're doing development, as part of their edit-compile-test cycle
> - analysis problems are reported immediately, and can be acted on
> immediately (e.g. by treating some checker tests as being hard errors).
>
> It would also be good to have a way to run analyzer(s) when packages are
> built, with a variety of precanned policies for analyzers.  For example,
> setuid binaries and network-facing daemons could each be built with a
> higher strictness of checking.
>
> It would also be good to tag binaries with information on what analyzers
> were run, what options they were invoked with, etc.
> Potentially have "dump_file" information from optimization passes stored
> in the metadata also.   Have a tool to query all of this.
>
> This way a distribution can perform a query like:
>
>   "show me all setuid binaries that contain code that wasn't checked
>with $CHECKER with $TEST set to be a hard error"
>
> Can/should we break the build if there are issues?
> Yes: but have a way to opt-in easily: if the tool is well-integrated with
> the
> compiler: e.g.
>
> -Wrun-analyzers=/usr/share/analyzers/userspace/network-facing-service
> then upstream developers and packagers can turn on the setting, and see
> what
> breaks, and fix it naturally within an compile-edit-test cycle
>
> This gives a relatively painless way to opt-in to increasing levels of
> strictness (e.g. by an upstream project, or by an individual developer).
>
> Does this slow the build down?
> Yes: but you can choose which analyzers run, and

Re: [PATCH 0/2] add unique_ptr class

[Trevor Saunders]

On Fri, Aug 04, 2017 at 04:43:32PM -0600, Martin Sebor wrote:
> On 07/31/2017 05:46 PM, tbsaunde+...@tbsaunde.org wrote:
> > From: Trevor Saunders 
> > 
> > Hi,
> > 
> > I've been saying I'd do this for a long time, but I'm finally getting to
> > importing the C++98 compatable unique_ptr class Pedro wrote for gdb a while
> > back.  I believe the gtl namespace also comes from Pedro, but GNU template
> > library seems as reasonable as any other name I can come up with.  I'm not 
> > sure
> > at the moment what outside of gcc may want to use this, but putting it 
> > include/
> > at least allows us to use it in libcpp which may be useful.  I didn't 
> > include
> > too much usage in this series, but I believe other people have wanted this 
> > too,
> > so I'm reasonably confident it will get a fair amount of usage.
> > 
> > patches individually bootstrapped + regtested on ppc64-linux-gnu, ok?
> 
> FWIW, I'm a big fan of RAII and I like the idea being able to
> rely on a smart pointer in GCC quite a bit.  Although I haven't
> reviewed the C++ 03 implementation in any detail I like how the
> new API is just transitional until GCC switches from C++ 98 to
> C++ 11 when it will be possible to drop it, presumably with no
> changes to client code.  (If there's any risk that clients might
> come to depend on the C++ 03 "features" or limitations in the
> meantime and thus jeopardize this goal I would only suggest to
> put some effort into making this harder.)

Given that stage 2 / 3 of a bootstrap is as C++14 I expect that it would
be pretty hard to put in code that works with the shim, but not real
unique_ptr, since in a bootstrap situation both would be used.

> Other than that, thank you for this nice enhancement!

your welcome

Trev

> 
> Martin
> 

Re: [PATCH 0/2] add unique_ptr class

[Trevor Saunders]

On Fri, Aug 04, 2017 at 07:52:18PM +0100, Jonathan Wakely wrote:
> On 31/07/17 19:46 -0400, tbsaunde+...@tbsaunde.org wrote:
> > I've been saying I'd do this for a long time, but I'm finally getting to
> > importing the C++98 compatable unique_ptr class Pedro wrote for gdb a while
> > back.  I believe the gtl namespace also comes from Pedro, but GNU template
> > library seems as reasonable as any other name I can come up with.
> 
> If it's inspired by "STL" then can we call it something else?
> 
> std::unique_ptr is not part of the STL, because the STL is a library
> of containers and algorithms from the 1990s. std::unique_ptr is
> something much newer that doesn't originate in the STL.

ok then I forgot or never knew that sorry.  It *is* std::unique_ptr in
C++11 or newer, so I think it would be odd to name it something else.

> STL != C++ Standard Library

Will try to remember that, sorry.

> If we want a namespace for GNU utilities, what's wrong with "gnu"?

I'm fine with s/gtl/gnu/g if that's what people want.

Trev

Re: [PATCH 1/2] add unique_ptr header

[Trevor Saunders]

On Fri, Aug 04, 2017 at 08:55:50PM +0100, Jonathan Wakely wrote:
> On 01/08/17 23:09 -0400, Trevor Saunders wrote:
> > aiui C++03 is C++98 with a few additions to the stl.
> 
> Again, STL != C++ Standard Library.
> 
> C++03 made some important changes to the core language, particularly
> the value-initialization rules.
> 
> Most of the library changes were small bug fixes, and most were to
> locales (which didn't originate in the STL and aren't even templates)
> and iostreams (which didn't originate in the STL).
> 
> There were also changes to std::auto_ptr (also not from the STL) which
> this might rely on (I haven't checked).

I doubt it, as Pedro said in his email it originally copied code from
std::auto_ptr, but it doesn't use the standard libraries definition of
std::auto_ptr anywhere.  However please do feel free to look at the
implementation.

Trev

> 
>