Re: [PATCH] Enable ICE-after-error with -fchecking
On Fri, 6 Dec 2019, Segher Boessenkool wrote: > On Wed, Dec 04, 2019 at 05:26:25PM +0100, Jakub Jelinek wrote: > > I'd fear people will use -fchecking and feel error-recovery bugs are then > > more important because they no longer get the more user friendly message. > > Can't we arrange for the emergency IL dump to happen in this case anyway? > > Does it not already happen? You need dump files enabled, of course. No, the issue is that IL verification errors use "error" and thus you trigger the ICE-after-error machinery which appearantly goes a completely different path. And -fchecking doesn't "disable" the ICE-after-error detection (which IMHO it should and what this patch is about). Richard.
Re: [PATCH 01/49] analyzer: user-facing documentation
On 11/15/19 6:22 PM, David Malcolm wrote: gcc/ChangeLog: * doc/invoke.texi ("Static Analyzer Options"): New list and new section. ("Warning Options"): Add static analysis warnings to the list. (-Wno-analyzer-double-fclose): New option. (-Wno-analyzer-double-free): New option. (-Wno-analyzer-exposure-through-output-file): New option. (-Wno-analyzer-file-leak): New option. (-Wno-analyzer-free-of-non-heap): New option. (-Wno-analyzer-malloc-leak): New option. (-Wno-analyzer-possible-null-argument): New option. (-Wno-analyzer-possible-null-dereference): New option. (-Wno-analyzer-null-argument): New option. (-Wno-analyzer-null-dereference): New option. (-Wno-analyzer-stale-setjmp-buffer): New option. (-Wno-analyzer-tainted-array-index): New option. (-Wno-analyzer-use-after-free): New option. (-Wno-analyzer-use-of-pointer-in-stale-stack-frame): New option. (-Wno-analyzer-use-of-uninitialized-value): New option. (-Wanalyzer-too-complex): New option. (-fanalyzer-call-summaries): New warning. (-fanalyzer-checker=): New warning. (-fanalyzer-fine-grained): New warning. (-fno-analyzer-state-merge): New warning. (-fno-analyzer-state-purge): New warning. (-fanalyzer-transitivity): New warning. (-fanalyzer-verbose-edges): New warning. (-fanalyzer-verbose-state-changes): New warning. (-fanalyzer-verbosity=): New warning. (-fdump-analyzer): New warning. (-fdump-analyzer-callgraph): New warning. (-fdump-analyzer-exploded-graph): New warning. (-fdump-analyzer-exploded-nodes): New warning. (-fdump-analyzer-exploded-nodes-2): New warning. (-fdump-analyzer-exploded-nodes-3): New warning. (-fdump-analyzer-supergraph): New warning. FAOD, I looked this over when the patch set first went by, but since it was marked as "RFC" I didn't do a detailed review. On first glance I thought the content is generally fine, but the organization could use some work. E.g. the -fdump-analyzer* options and things that mess with internal parameters probably belong in the GCC Developer Options section, and I'd like to introduce some substructure into the Warning Options section instead of adding a new section at the same level. I can take another look when the patches are closer to being ready to commit. -Sandra
Re: [PATCH] Enable mask operation for 128/256-bit vector VCOND_EXPR under avx512f (PR92686)
On Thu, Dec 5, 2019 at 4:03 PM Jakub Jelinek wrote: > > On Thu, Dec 05, 2019 at 09:56:46AM +0800, Hongtao Liu wrote: > > --- a/gcc/config/i386/i386-expand.c > > +++ b/gcc/config/i386/i386-expand.c > > + /* Using vector move with mask register. */ > > + cmp = force_reg (cmpmode, cmp); > > + /* Optimize for mask zero. */ > > + op_true = > > + op_true != CONST0_RTX (mode) ? force_reg (mode, op_true) : op_true; > > + op_false = > > + op_false != CONST0_RTX (mode) ? force_reg (mode, op_false) : op_false; > > The above two still aren't correct, = doesn't belong at the end of line > either. > > op_true > = op_true != CONST0_RTX (mode) ? force_reg (mode, op_true) : op_true; > > would be ok, > > op_false > = op_false != CONST0_RTX (mode) ? force_reg (mode, op_false) : > op_false; > > is too long, so e.g. > > op_false = (op_false != CONST0_RTX (mode) > ? force_reg (mode, op_false) : op_false); > > > + /* Reverse op_true op_false. */ > > + n = op_true; > > + op_true = op_false; > > + op_false = n; > > Please use > std::swap (op_true, op_false); > instead of the above 3 lines. > > Also, can you please add at least one testcase for this with -masm=intel, > effective target masm_intel and dg-do assemble to make sure it assembles? > Perhaps just one -mavx512vl -mavx512bw avx512vl/avx512bw effective target that > tests all the patterns? > Yes, avx512vl-pr92686-vpcmp-intelasm-1.c, avx512bw-pr92686-vpcmp-intelasm-1.c are added. > Ok with those changes. > > Jakub > Committed, thanks. -- BR, Hongtao
libgo patch committed: Hurd fix
I've committed this Hurd fix by Samuel Thibault for GCC PR 92861. Ian Index: gcc/go/gofrontend/MERGE === --- gcc/go/gofrontend/MERGE (revision 279063) +++ gcc/go/gofrontend/MERGE (working copy) @@ -1,4 +1,4 @@ -15c7bc9f0a432bc09716758412ea41d6caa6491b +1da5ceb8daaab7a243fffd6a647554cf674716f8 The first line of this file holds the git revision number of the last merge done from the gofrontend repository. Index: libgo/go/runtime/os_hurd.go === --- libgo/go/runtime/os_hurd.go (revision 279062) +++ libgo/go/runtime/os_hurd.go (working copy) @@ -2,7 +2,7 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -// This file is derived from os_solaris.go. +// This file is derived from os_aix.go. package runtime @@ -37,6 +37,10 @@ func sem_post(sem *_sem_t) int32 //extern sem_timedwait func sem_timedwait(sem *_sem_t, timeout *timespec) int32 +//go:noescape +//extern clock_gettime +func clock_gettime(clock_id int32, timeout *timespec) int32 + //go:nosplit func semacreate(mp *m) { if mp.waitsema != 0 { @@ -60,7 +64,23 @@ func semasleep(ns int64) int32 { _m_ := getg().m if ns >= 0 { var ts timespec - ts.setNsec(ns) + + if clock_gettime(_CLOCK_REALTIME, &ts) != 0 { + throw("clock_gettime") + } + + sec := int64(ts.tv_sec) + ns/1e9 + nsec := int64(ts.tv_nsec) + ns%1e9 + if nsec >= 1e9 { + sec++ + nsec -= 1e9 + } + if sec != int64(timespec_sec_t(sec)) { + // Handle overflows (timespec_sec_t is 32-bit in 32-bit applications) + sec = 1<<31 - 1 + } + ts.tv_sec = timespec_sec_t(sec) + ts.tv_nsec = timespec_nsec_t(nsec) if sem_timedwait((*_sem_t)(unsafe.Pointer(_m_.waitsema)), &ts) != 0 { err := errno() @@ -105,3 +125,7 @@ func osinit() { physPageSize = uintptr(getPageSize()) } } + +const ( + _CLOCK_REALTIME = 0 +)
Re: [PATCH 32/49] analyzer: new files: pending-diagnostic.{cc|h}
On Sun, 2019-12-08 at 09:43 -0500, David Malcolm wrote: > On Sat, 2019-12-07 at 08:05 -0700, Jeff Law wrote: > > On Fri, 2019-11-15 at 20:23 -0500, David Malcolm wrote: > > > This patch adds classes used by the analyzer for handling its > > > diagnostics > > > (queueing them, deduplicating them, precision-of-wording hooks). > > > > > > gcc/ChangeLog: > > > * analyzer/pending-diagnostic.cc: New file. > > > * analyzer/pending-diagnostic.h: New file. > > Definitely want to be using this elsewhere. So again, perhaps move > > out > > of the analyzer into a generic location? > > > > Jeff > > This one I'd prefer to keep in the analyzer subdir, as it actually > uses > some analyzer-specific types, for the precision-of-wording vfuncs; > this > is the interface within the analyzer between the analyzer's > path/event > building logic [1] and the specific descriptions implemented in the > various sm-* files. There may be parts we want to generalize, particularly with the introduction of __bulitin_warning. But if you want to defer making this infrastructure a bit more generic now, I won't strenously object. jeff >
[committed] Add a bswap testcase
Hi, While testing a GCC patch (which I am not ready to submit yet), I wrote a testcase which had produced wrong code (with an earlier version of the patch). There was no other testcase for it in the testsuite so I adding one. This is a reduced testcase from the uboot PCIe code. Thanks, Andrew Pinski * gcc.c-torture/execute/bswap-3.c: New testcase. Index: ChangeLog === --- ChangeLog (revision 279099) +++ ChangeLog (working copy) @@ -1,3 +1,7 @@ +2019-12-08 Andrew Pinski + + * gcc.c-torture/execute/bswap-3.c: New test. + 2019-12-08 Sandra Loosemore Revert: Index: gcc.c-torture/execute/bswap-3.c === --- gcc.c-torture/execute/bswap-3.c (nonexistent) +++ gcc.c-torture/execute/bswap-3.c (working copy) @@ -0,0 +1,25 @@ +/* { dg-require-effective-target int32plus } */ + +int f(unsigned int a) __attribute__((noipa)); +int f(unsigned int a) +{ + return ((__builtin_bswap32(a))>>24) & 0x3; +} + + +int g(unsigned int a) __attribute__((noipa)); +int g(unsigned int a) +{ + return a&0x3; +} + +int main(void) +{ + for (int b = 0; b <= 0xF; b++) +{ + if (f(b) != g(b)) + __builtin_abort (); +} + return 0; +} +
Re: Fix overflows in -fprofile-reorder-functions
> 2On Sun, 8 Dec 2019, Jan Hubicka wrote: > > > Other explanation would be that our new qsort with broken comparator due to > > overflow can actualy remove some entries in the array, but that sounds bit > > crazy. > > gcc_qsort only reorders elements, making it possible for gcc_qsort_chk (that > runs afterwards) to catch crazy comparators in a sound manner. > > > Bootstrapped/regested x86_64-linux. Comitted. > > I have a few comments, please see below. Thanks. I will revisit the patch tomorrow - as mentioned in the other mail I got overzelaous about the 64bit support - we can easily sort out too large numbers as broken profile. This was end of very long debugging session and I should have tought it out better. Honza
Re: Fix overflows in -fprofile-reorder-functions
On Dez 08 2019, Jan Hubicka wrote: > Index: cgraphunit.c > === > --- cgraphunit.c (revision 279076) > +++ cgraphunit.c (working copy) > @@ -2359,19 +2359,33 @@ cgraph_node::expand (void) > /* Node comparator that is responsible for the order that corresponds > to time when a function was launched for the first time. */ > > -static int > -node_cmp (const void *pa, const void *pb) > +int > +tp_first_run_node_cmp (const void *pa, const void *pb) > { >const cgraph_node *a = *(const cgraph_node * const *) pa; >const cgraph_node *b = *(const cgraph_node * const *) pb; > + gcov_type tp_first_run_a = a->tp_first_run; > + gcov_type tp_first_run_b = b->tp_first_run; > + > + if (!opt_for_fn (a->decl, flag_profile_reorder_functions) > + || a->no_reorder) > +tp_first_run_a = 0; > + if (!opt_for_fn (b->decl, flag_profile_reorder_functions) > + || b->no_reorder) > +tp_first_run_b = 0; > + > + if (tp_first_run_a == tp_first_run_b) > +return b->order - a->order; > >/* Functions with time profile must be before these without profile. */ > - if (!a->tp_first_run || !b->tp_first_run) > -return a->tp_first_run - b->tp_first_run; > + if (!tp_first_run_a || !tp_first_run_b) > +return tp_first_run_a ? 1 : -1; > > - return a->tp_first_run != b->tp_first_run > - ? b->tp_first_run - a->tp_first_run > - : b->order - a->order; > + /* Watch for overlflow - tp_first_run is 64bit. */ overflow Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 "And now for something completely different."
Re: Fix overflows in -fprofile-reorder-functions
2On Sun, 8 Dec 2019, Jan Hubicka wrote: > Other explanation would be that our new qsort with broken comparator due to > overflow can actualy remove some entries in the array, but that sounds bit > crazy. gcc_qsort only reorders elements, making it possible for gcc_qsort_chk (that runs afterwards) to catch crazy comparators in a sound manner. > Bootstrapped/regested x86_64-linux. Comitted. I have a few comments, please see below. > --- cgraphunit.c (revision 279076) > +++ cgraphunit.c (working copy) > @@ -2359,19 +2359,33 @@ cgraph_node::expand (void) > /* Node comparator that is responsible for the order that corresponds > to time when a function was launched for the first time. */ > > -static int > -node_cmp (const void *pa, const void *pb) > +int > +tp_first_run_node_cmp (const void *pa, const void *pb) > { >const cgraph_node *a = *(const cgraph_node * const *) pa; >const cgraph_node *b = *(const cgraph_node * const *) pb; > + gcov_type tp_first_run_a = a->tp_first_run; > + gcov_type tp_first_run_b = b->tp_first_run; > + > + if (!opt_for_fn (a->decl, flag_profile_reorder_functions) > + || a->no_reorder) > +tp_first_run_a = 0; > + if (!opt_for_fn (b->decl, flag_profile_reorder_functions) > + || b->no_reorder) > +tp_first_run_b = 0; > + > + if (tp_first_run_a == tp_first_run_b) > +return b->order - a->order; > >/* Functions with time profile must be before these without profile. */ > - if (!a->tp_first_run || !b->tp_first_run) > -return a->tp_first_run - b->tp_first_run; > + if (!tp_first_run_a || !tp_first_run_b) > +return tp_first_run_a ? 1 : -1; The code does the opposite of the comment: when tp_first_run_b is 0, it will return 1, indicating a > b, causing b to appear in front of a in the sorted array. I would recommend to make these variables uint64_t, then you can simply do tp_first_run_a--; tp_first_run_b--; making 0 wrap around to UINT64_MAX. Then they will naturally sort after all other nodes. > - return a->tp_first_run != b->tp_first_run > - ? b->tp_first_run - a->tp_first_run > - : b->order - a->order; > + /* Watch for overlflow - tp_first_run is 64bit. */ > + if (tp_first_run_a > tp_first_run_b) > +return -1; > + else > +return 1; This also sorts in the reverse order -- please fix the comments if that's really intended. > + /* Output functions in RPO so callers get optimized before callees. This > + makes ipa-ra and other propagators to work. > + FIXME: This is far from optimal code layout. */ I think this should have said "callees get optimized before callers". Thanks. Alexander
Re: Fix overflows in -fprofile-reorder-functions
> Hi, > this patch fixes three sissues with -fprofile-reorder-functions: > 1) First is that tp_first_run is stored as 32bit integer while it can easily >overflow (and does so during Firefox profiling). Actually the overflow problem is possible only with mismatched profiles (which does happen for me). This is because time is increased only if new function is found, so it can not get over 2^32 for programs that do not have more than 2^32 functions. So thinking this over, perhaps it is better to keep tp_first_run 32bit and cap it on profile corruption. I will look into that tomorrow. The other two problems are valid though, so i will fix this incrementally. Honza
Re: [C++ Patch] Improve build_*_cast locations
On 12/6/19 4:36 PM, Paolo Carlini wrote: Hi, On 06/12/19 18:53, Jason Merrill wrote: On 12/6/19 11:14 AM, Paolo Carlini wrote: Hi, as promised, the below takes care of the various remaining casts. It's mostly mechanical, follows the same approach used for build_functional_cast. Tested x86_64-linux. It occurs to me that once we're passing the location into the build_* functions, we can apply it to the expression there rather than in the parser. In fact that occurred to me too ;) but at first I didn't like the idea of multiplying the set_location calls... Anyway, in practice for the casts of this last patch the idea works reasonably well, because most of the complexity is encapsulated in the *_1 versions of the functions and the build_* functions proper have only a couple of returns. I wanted to send those changes... But then my build failed in the libcp1plugin.cc bits because the switch also includes c_cast and all the functions must return the same type. And then for consistency we want also to adjust functional_cast (which allows to remove the set_location in the parser which also remained after my previous patch). Those two cast however are more annoying, because there aren't *_1 counterparts and the functions have *lots* of returns, are complicated. Thus for those I'm proposing to create ad hoc *_1 identical to the current functions in order to have single set_location in cp_build_c_cast and build_functional_cast. All in all, I don't have better ideas... I'm finishing testing the below. Hmm, is the change to cp_expr really necessary vs. using protected_set_expr_location? Jason
Re: [nios2, committed] Disable --eh-frame-hdr with -pie or -shared on nios2-linux-gnu
On 12/5/19 2:47 PM, Sandra Loosemore wrote: I've checked in this patch to fix a long-standing bug on nios2-linux-gnu that resulted in linker errors like ../nios2-linux-gnu/bin/ld: FDE encoding in /tmp/cccfpQ2l.o(.eh_frame) prevents .eh_frame_hdr table being created when linking with -pie or -shared. The nios2 ABI doesn't provide the relocations needed for those situations, so we shouldn't be passing --eh-frame-hdr to the linker. I've also checked in a related patch to the linker: it was emitting these diagnostics unconditionally, even when --eh-frame-hdr was not enabled. See https://sourceware.org/ml/binutils/2019-12/msg00013.html -Sandra I've reverted this patch, as it appears I accidentally tested a different version of it (that disabled the option for -pie only and not -shared) than I committed, and the one committed triggers other errors. I need to revisit my original analysis of the linker bug and figure out why it is being triggered for -shared too. And there's probably another bug lurking somewhere that's causing the execution errors in shared libraries linked without --eh-frame-hdr. Needs More Thought. BTW, the test case for -shared is the C++ version of the program used to test for { dg-require-effective-target shared } support. :-( (But libstdc++ builds fine as a shared library and doesn't fall over during testing) -Sandra
Fix overflows in -fprofile-reorder-functions
Hi, this patch fixes three sissues with -fprofile-reorder-functions: 1) First is that tp_first_run is stored as 32bit integer while it can easily overflow (and does so during Firefox profiling). 2) Second problem is that flag_profile_functions can not be tested w/o function context. The changes to expand_all_functions makes it to work on mixed units by first outputting all functions w/o -fprofile-reorder-function (or with no profile info) and then outputting in first_run order 3) LTO partitioner was mixing up order by tp_first_run and by order. for no_reorder we definitly want to order via first, while for everything else we want to roder by second. I have also merged duplicated comparators since they are bit fragile into tp_first_run_node_cmp. I originaly started to look into this because of undefined symbols with Firefox PGO builds. These symbols went away with fixing these bug but I am not quite sure how. it is possible that there is another problem in lto_blanced_map but even after reading the noreorder code few times carefuly I did not find it. Other explanation would be that our new qsort with broken comparator due to overflow can actualy remove some entries in the array, but that sounds bit crazy. Bootstrapped/regested x86_64-linux. Comitted. * cgraph.c (cgraph_node::dump): Make tp_first_run 64bit. * cgraph.h (cgrpah_node): Likewise. (tp_first_run_node_cmp): Deeclare. * cgraphunit.c (node_cmp): Rename to ... (tp_first_run_node_cmp): ... this; export; watch for 64bit overflows; clear tp_first_run for no_reorder and !flag_profile_reorder_functions. (expand_all_functions): Collect tp_first_run and normal functions to two vectors so the other functions remain sorted. Do not check for flag_profile_reorder_functions it is function local flag. * profile.c (compute_value_histograms): Update tp_first_run printing. * lto-partition.c (node_cmp): Turn into simple order comparsions. (varpool_node_cmp): Remove. (add_sorted_nodes): Use node_cmp. (lto_balanced_map): Use tp_first_run_node_cmp. Index: cgraph.c === --- cgraph.c(revision 279076) +++ cgraph.c(working copy) @@ -1954,7 +1954,7 @@ cgraph_node::dump (FILE *f) count.dump (f); } if (tp_first_run > 0) -fprintf (f, " first_run:%i", tp_first_run); +fprintf (f, " first_run:%" PRId64, (int64_t) tp_first_run); if (origin) fprintf (f, " nested in:%s", origin->asm_name ()); if (gimple_has_body_p (decl)) Index: cgraph.h === --- cgraph.h(revision 279076) +++ cgraph.h(working copy) @@ -1430,6 +1430,8 @@ struct GTY((tag ("SYMTAB_FUNCTION"))) cg /* Expected number of executions: calculated in profile.c. */ profile_count count; + /* Time profiler: first run of function. */ + gcov_type tp_first_run; /* How to scale counts at materialization time; used to merge LTO units with different number of profile runs. */ int count_materialization_scale; @@ -1437,8 +1439,6 @@ struct GTY((tag ("SYMTAB_FUNCTION"))) cg unsigned int profile_id; /* ID of the translation unit. */ int unit_id; - /* Time profiler: first run of function. */ - int tp_first_run; /* Set when decl is an abstract function pointed to by the ABSTRACT_DECL_ORIGIN of a reachable function. */ @@ -2463,6 +2463,7 @@ cgraph_inline_failed_type_t cgraph_inlin /* In cgraphunit.c */ void cgraphunit_c_finalize (void); +int tp_first_run_node_cmp (const void *pa, const void *pb); /* Initialize datastructures so DECL is a function in lowered gimple form. IN_SSA is true if the gimple is in SSA. */ Index: cgraphunit.c === --- cgraphunit.c(revision 279076) +++ cgraphunit.c(working copy) @@ -2359,19 +2359,33 @@ cgraph_node::expand (void) /* Node comparator that is responsible for the order that corresponds to time when a function was launched for the first time. */ -static int -node_cmp (const void *pa, const void *pb) +int +tp_first_run_node_cmp (const void *pa, const void *pb) { const cgraph_node *a = *(const cgraph_node * const *) pa; const cgraph_node *b = *(const cgraph_node * const *) pb; + gcov_type tp_first_run_a = a->tp_first_run; + gcov_type tp_first_run_b = b->tp_first_run; + + if (!opt_for_fn (a->decl, flag_profile_reorder_functions) + || a->no_reorder) +tp_first_run_a = 0; + if (!opt_for_fn (b->decl, flag_profile_reorder_functions) + || b->no_reorder) +tp_first_run_b = 0; + + if (tp_first_run_a == tp_first_run_b) +return b->order - a->order; /* Functions with time profile must be before these without profile. */ - if (!a->tp_first_run || !b->tp_first_run) -return a->tp_first_run - b->tp_first_run; + if (!t
Fix tp_first_run update in split_function
Hi, the value 0 in tp_first_run is special meaing that profile is unknown. We should not set it to 1. Bootstrapped/regtested x86_64-linux, comitted. * ipa-split.c (split_function): Preserve 0 tp_first_run. Index: ipa-split.c === --- ipa-split.c (revision 279076) +++ ipa-split.c (working copy) @@ -1369,7 +1369,8 @@ split_function (basic_block return_bb, c /* Let's take a time profile for splitted function. */ - node->tp_first_run = cur_node->tp_first_run + 1; + if (cur_node->tp_first_run) +node->tp_first_run = cur_node->tp_first_run + 1; /* For usual cloning it is enough to clear builtin only when signature changes. For partial inlining we however cannot expect the part
Re: [PATCH 32/49] analyzer: new files: pending-diagnostic.{cc|h}
On Sat, 2019-12-07 at 08:05 -0700, Jeff Law wrote: > On Fri, 2019-11-15 at 20:23 -0500, David Malcolm wrote: > > This patch adds classes used by the analyzer for handling its > > diagnostics > > (queueing them, deduplicating them, precision-of-wording hooks). > > > > gcc/ChangeLog: > > * analyzer/pending-diagnostic.cc: New file. > > * analyzer/pending-diagnostic.h: New file. > Definitely want to be using this elsewhere. So again, perhaps move > out > of the analyzer into a generic location? > > Jeff This one I'd prefer to keep in the analyzer subdir, as it actually uses some analyzer-specific types, for the precision-of-wording vfuncs; this is the interface within the analyzer between the analyzer's path/event building logic [1] and the specific descriptions implemented in the various sm-* files. Dave [1] which is specific to the analyzer, although it hands off types that can be consumed by the generic diagnostic subsystem.
Re: [PATCH 24/49] analyzer: new file: analyzer-pass.cc
On Sat, 2019-12-07 at 07:51 -0700, Jeff Law wrote: > On Fri, 2019-11-15 at 20:23 -0500, David Malcolm wrote: > > This patch adds the IPA pass boilerplate for the analyzer. > > > > gcc/ChangeLog: > > * analyzer/analyzer-pass.cc: New file. > Nothing I see controversial here. But obviously will need some > adjustment if we're moving away from using the plugin facilities. > > jeff Indeed, in v3 it gains a "gate" vfunc, uses a timevar, and the "sorry" for not-enabled-at-configure-time moves from the driver to this pass's execute "vfunc": https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00531.html Dave
Re: [PATCH 22/49] analyzer: params.def: new parameters
On Sun, 2019-12-08 at 00:42 -0500, Eric Gallager wrote: > On 12/7/19, Jeff Law wrote: > > On Fri, 2019-11-15 at 20:23 -0500, David Malcolm wrote: > > > gcc/ChangeLog: > > > * params.def (PARAM_ANALYZER_BB_EXPLOSION_FACTOR): New param. > > > (PARAM_ANALYZER_MAX_ENODES_PER_PROGRAM_POINT): New param. > > > (PARAM_ANALYZER_MAX_RECURSION_DEPTH): New param. > > > (PARAM_ANALYZER_MIN_SNODES_FOR_CALL_SUMMARY): New param. > > Doesn't this need a rethink/reimplementation after M. Liska's > > changes? > > > > jeff > > > > > > I think he did that in a followup patch from a later patchset... Yes: https://gcc.gnu.org/ml/gcc-patches/2019-11/msg02030.html
Re: [PATCH 16/49] Add support for in-tree plugins
On Sat, 2019-12-07 at 07:39 -0700, Jeff Law wrote: > On Fri, 2019-11-15 at 20:23 -0500, David Malcolm wrote: > > This patch adds support for "in-tree" plugins i.e. GCC plugins that > > live > > in the GCC source tree and are shipped as part of the GCC tarball. > > > > The patch adds Makefile/configure machinery for handling in-tree > > GCC > > plugins, adapted from how we support frontends. > > > > Each in-tree plugin should provide a Make-plugin.in and config- > > plugin.in, > > analogous to the Make-lang.in and config-lang.in provided by a > > frontend; > > they can also supply options via a plugin.opt, analogous to a > > frontend's > > lang.opt. > > > > The patch adds a --enable-plugins=[LIST OF PLUGIN NAMES] configure > > option, > > analogous to --enable-languages. The default is for no such > > plugins > > to be > > enabled. > > > > ChangeLog: > > * configure.ac: Handle --enable-plugins. > > > > gcc/ChangeLog: > > * Makefile.in (CONFIG_PLUGINS): New. > > (SUBDIRS, subdirs): Generalize to cover plugins as well as > > languages. > > (plugin_opt_files): New. > > (ALL_OPT_FILES): Add plugin_opt_files. > > (ALL_HOST_PLUGIN_OBJS): New. > > (ALL_HOST_OBJS): Add ALL_HOST_PLUGIN_OBJS. > > (plugin_hooks): New. > > (PLUGIN_MAKEFRAGS): New; include them. > > (Makefile): Add dependency on PLUGIN_MAKEFRAGS. > > (all.cross): Add dependency on plugin.all.cross. > > (start.encap): Add plugin.start.encap. > > (rest.encap): Add plugin.rest.encap. > > (SELFTEST_TARGETS): Add selftest_plugins. > > (info): Add dependency on lang.info. > > (dvi): Add dependency on plugin.dvi. > > (pdf): Add dependency on plugin.pdf. > > (HTMLS_BUILD): Add plugin.html. > > (man): Add plugin.man. > > (mostlyclean): Add plugin.mostlyclean. > > (clean): Add plugin.clean. > > (distclean): Update for renaming of Make-hooks to Make-lang- > > hooks; > > add Make-plugin-hooks. Add plugin.distclean dependency. > > (maintainer-clean): Add plugin.maintainer-clean. > > (install-plugin): Add plugin.install-plugin. > > (install-common): Add plugin.install-common. > > (install-info): Add plugin.install-info. > > (install-pdf): Add plugin.install-pdf. > > (install-html): Add plugin.install-html. > > (install-man): Add plugin.install-man. > > (uninstall): Add plugin.uninstall. > > (TAGS): Add plugin.tags. > > * configure.ac (Make-hooks): Rename to Make-lang-hooks. > > (plugin_opt_files): New. > > (plugin_specs_files): New. > > (plugin_tree_files): New. > > (all_plugins): New. > > (all_plugin_makefrags): New. > > (all_selected_plugins): New. > > (plugin_hooks): New. > > ("Plugin hooks"): New section. Iterate through config- > > plugin.in, > > analogously to config-lang.in. > > (check_plugins): New. > > (selftest_plugins): New. > > (Make-plugin-hooks): Emit. > > * doc/install.texi (--enable-plugins): New option. > So do we want to push on the concept of in-tree plugins for somethign > like annobin? If not, given the general desire to not use plugins > for > teh analyzer, would we still want this? > > jeff As this becomes orthogonal to the analyzer work, I've dropped this patch from v3 of the kit, but someone else is welcome to pick this up and run with it. Dave
Re: [PATCH 06/49] timevar.h: add auto_client_timevar class
On Sat, 2019-12-07 at 07:29 -0700, Jeff Law wrote: > On Fri, 2019-11-15 at 20:22 -0500, David Malcolm wrote: > > This patch adds a class "auto_client_timevar", similar to > > auto_timevar, > > but for plugins to use on their own timing items that aren't in > > timevar.def > > > > gcc/ChangeLog: > > * timevar.h (class auto_client_timevar): New class. > So if we move away from a plug-in architecture to an integrated first > class analyzer, do we still want this change? I've dropped this change in the v3 of the kit, in favor of new TV_* values; see: https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00529.html Dave
[PATCH 1/2] Fixup for rebase: c-format.c: get_pointer_to_named_type -> get_named_type
r278634 changed this, so need to apply this when rebasing gcc/c-family/ChangeLog: * c-format.c (init_dynamic_diag_info): Use get_named_type rather than get_pointer_to_named_type, reflecting r278634. --- gcc/c-family/c-format.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gcc/c-family/c-format.c b/gcc/c-family/c-format.c index c5ca71216633..be092b1f3b62 100644 --- a/gcc/c-family/c-format.c +++ b/gcc/c-family/c-format.c @@ -4993,7 +4993,7 @@ init_dynamic_diag_info (void) /* Similar to the above but for diagnostic_event_id_t*. */ if (!local_event_ptr_node || local_event_ptr_node == void_type_node) -local_event_ptr_node = get_pointer_to_named_type ("diagnostic_event_id_t"); +local_event_ptr_node = get_named_type ("diagnostic_event_id_t"); static tree hwi; -- 2.21.0
[PATCH 2/2] Rework as a non-plugin
gcc/ChangeLog: * Makefile.in (lang_opt_files): Add analyzer.opt. (ANALYZER_OBJS): New. (OBJS): Add ANALYZER_OBJS. * analyzer/Make-plugin.in: Delete file. * analyzer/analysis-plan.cc: Rework headers. Guard with #if ENABLE_ANALYZER. (analysis_plan::analysis_plan): Convert from auto_client_timevar to auto_timevar. * analyzer/analyzer-logging.cc: Guard with #if ENABLE_ANALYZER. * analyzer/analyzer-pass.cc: Rework headers. Guard with #if ENABLE_ANALYZER. (pass_data_analyzer): Use TV_ANALYZER. (pass_analyzer::gate): New. (pass_analyzer::execute): Move the check for #if ENABLE_ANALYZER here. (make_pass_analyzer): Make non-static. (register_analyzer_pass): Delete. * analyzer/analyzer-plugin.cc: Delete file. * analyzer/analyzer-selftests.cc: Rework headers. (selftest::run_analyzer_selftests): Guard body with #if ENABLE_ANALYZER. * analyzer/analyzer-selftests.h: Fix comment. * analyzer/analyzer.cc: Rework headers. Guard with #if ENABLE_ANALYZER. * analyzer/analyzer.h: Include "function.h". * analyzer/call-string.cc: Rework headers. Guard with #if ENABLE_ANALYZER. * analyzer/checker-path.cc: Rework headers. Guard with #if ENABLE_ANALYZER. * analyzer/config-plugin.in: Delete file. * analyzer/constraint-manager.cc: Rework headers. Guard with #if ENABLE_ANALYZER. * analyzer/diagnostic-manager.cc: Rework headers. Guard with #if ENABLE_ANALYZER. (diagnostic_manager::emit_saved_diagnostics): Convert from auto_client_timevar to auto_timevar. * analyzer/digraph.cc: Rework headers. Guard with #if ENABLE_ANALYZER. * analyzer/engine.cc: Rework headers. Guard with #if ENABLE_ANALYZER. (strongly_connected_components::strongly_connected_components): Convert from auto_client_timevar to auto_timevar. (exploded_graph::process_worklist): Likewise. (exploded_graph::dump_exploded_nodes): Likewise. (dump_callgraph): Likewise. (impl_run_checkers): Likewise. (run_checkers): Drop top-level auto_client_timevar in favor of the tv_id of the pass as a whole. * analyzer/exploded-graph.h: Include "alloc-pool.h". * analyzer/graphviz.cc: Rework headers. Guard with #if ENABLE_ANALYZER. * analyzer/pending-diagnostic.cc: Rework headers. Guard with #if ENABLE_ANALYZER. * analyzer/program-point.cc: Rework headers. Guard with #if ENABLE_ANALYZER. * analyzer/program-state.cc: Rework headers. Guard with #if ENABLE_ANALYZER. * analyzer/region-model.cc: Rework headers. Guard with #if ENABLE_ANALYZER. (dump_tree): New function. (region_model::dump_to_pp): Use dump_tree rather than %E. (dump_vec_of_tree): Likewise. (region_model::dump_summary_of_map): Likewise. * analyzer/region-model.h: Include "sbitmap.h". * analyzer/shortest-paths.h (shortest_paths::shortest_paths): Convert from auto_client_timevar to auto_timevar. * analyzer/sm-file.cc: Rework headers. Guard with #if ENABLE_ANALYZER. * analyzer/sm-malloc.cc: Rework headers. Guard with #if ENABLE_ANALYZER. * analyzer/sm-pattern-test.cc: Rework headers. Guard with #if ENABLE_ANALYZER. * analyzer/sm-sensitive.cc: Rework headers. Guard with #if ENABLE_ANALYZER. * analyzer/sm-signal.cc: Rework headers. Guard with #if ENABLE_ANALYZER. * analyzer/sm-taint.cc: Rework headers. Guard with #if ENABLE_ANALYZER. * analyzer/sm.cc: Rework headers. Guard with #if ENABLE_ANALYZER. * analyzer/state-purge.cc: Rework headers. Guard with #if ENABLE_ANALYZER. (state_purge_map::state_purge_map): Convert from auto_client_timevar to auto_timevar. * analyzer/supergraph.cc: Rework headers. Guard with #if ENABLE_ANALYZER. (supergraph::supergraph): Convert from auto_client_timevar to auto_timevar. * analyzer/supergraph.h: Include "options.h", "function.h", "cfg.h", and "basic-block.h". * analyzer/tristate.cc: Rework headers. Guard with #if ENABLE_ANALYZER. * common.opt (fanalyzer): Convert from Driver to Common. Add description. * configure.ac (--disable-analyzer, ENABLE_ANALYZER): New option. (gccdepdir): Also created depdir for "analyzer" subdir. * gcc.c (driver_handle_option): Drop case OPT_fanalyzer. (process_command): Drop handling of flag_analyzer; this is now done in the gate vfunc of the pass. * passes.def (pass_analyzer): Add before pass_ipa_whole_program_visibility. * selftest-run-tests.c: Include "a
[PATCH 0/2] v3 of analyzer patch kit (unsquashed)
I've most of the way through a v3 of the patch kit, but given the ongoing discussion/review on this mailing list, I thought I'd post what I have so far. I'm going to work on squashing the changes, to produce a v3 relative to trunk. This drops the "in-tree plugin" idea, and the auto_client_timevar, but in the meantime here are the changes relative to the branch. I've rebased from r278495 (2019-11-20) to r279013 (2019-12-05). Patch 1 contains the only change needed by the rebase. Patch 2 contains the fixups *relative to the analyzer branch* to convert it from being an in-tree plugin to being built into gcc, with an opt-out configure option (--disable-analyzer). Changes include: * I kept the new source files in the new "gcc/analyzer" subdirectory, although they're still built by gcc/Makefile.in - we have a huge number of source files in the "gcc" subdir, and we already have subdirs that complicate grep, so keeping a subdir for the source files seemed cleanest. (though some e.g. tristate.h/cc might be better in the gcc subdir, as noted in review) * I kept a separate .opt file for the new command line options * I dropped #include "gcc-plugin.h" from the source files, and add includes of the things that are needed (which was rather tedious as e.g. gimple.h doesn't include what it needs, requiring manually included its dependences everywhere, which seems like a violation of DRY - but that's an orthogonal issue) * I added #if ENABLE_ANALYZER guards around the bodies of the source files so that it ought to compile away with --disable-analyzer I haven't yet measured the effect on * I dropped the auto_client_timevar in favor of new TV_*. * Putting everything in the core middle-end exposed a breakage in the selftests for C++ due to the %E in some dumps, which is FE-specific. I fixed this by rewriting the dumps to avoid %E. See the ChangeLog of patch 2 for more details. Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu. Pushed to branch "dmalcolm/analyzer-v3-unsquashed" on the GCC git mirror. I may integrate Lewis's fix for multicolumn characters into trunk first, then rebase my patch, since our changes to diagnostic-show-locus.c have numerous conflicts, albeit ones that ought to be mechanical (if tedious) to resolve. Dave David Malcolm (2): Fixup for rebase: c-format.c: get_pointer_to_named_type -> get_named_type Rework as a non-plugin gcc/Makefile.in | 32 +++- gcc/analyzer/Make-plugin.in | 182 -- gcc/analyzer/analysis-plan.cc | 8 +- gcc/analyzer/analyzer-logging.cc | 4 + gcc/analyzer/analyzer-pass.cc | 41 +++-- gcc/analyzer/analyzer-plugin.cc | 63 gcc/analyzer/analyzer-selftests.cc| 3 +- gcc/analyzer/analyzer-selftests.h | 2 +- gcc/analyzer/analyzer.cc | 7 +- gcc/analyzer/analyzer.h | 2 + gcc/analyzer/{plugin.opt => analyzer.opt} | 0 gcc/analyzer/call-string.cc | 6 +- gcc/analyzer/checker-path.cc | 8 +- gcc/analyzer/config-plugin.in | 34 gcc/analyzer/constraint-manager.cc| 10 +- gcc/analyzer/diagnostic-manager.cc| 7 +- gcc/analyzer/digraph.cc | 5 +- gcc/analyzer/engine.cc| 26 ++-- gcc/analyzer/exploded-graph.h | 1 + gcc/analyzer/graphviz.cc | 5 +- gcc/analyzer/pending-diagnostic.cc| 5 +- gcc/analyzer/program-point.cc | 5 +- gcc/analyzer/program-state.cc | 5 +- gcc/analyzer/region-model.cc | 66 gcc/analyzer/region-model.h | 1 + gcc/analyzer/shortest-paths.h | 2 +- gcc/analyzer/sm-file.cc | 8 +- gcc/analyzer/sm-malloc.cc | 9 +- gcc/analyzer/sm-pattern-test.cc | 7 +- gcc/analyzer/sm-sensitive.cc | 9 +- gcc/analyzer/sm-signal.cc | 7 +- gcc/analyzer/sm-taint.cc | 8 +- gcc/analyzer/sm.cc| 8 +- gcc/analyzer/state-purge.cc | 10 +- gcc/analyzer/supergraph.cc| 9 +- gcc/analyzer/supergraph.h | 4 + gcc/analyzer/tristate.cc | 5 +- gcc/c-family/c-format.c | 2 +- gcc/common.opt| 3 +- gcc/configure | 25 ++- gcc/configure.ac | 14 +- gcc/gcc.c | 12 -- gcc/passes.def| 1 + gcc/selftest-run-tests.c | 4 + gcc/timevar.def | 11 ++ gcc/tree-pass.h | 1 + 46 files changed, 301 insertions(+), 386 deletions(-) delete mode 100644 gcc/analyzer/Make-plugin.in delete
Silence overactive sanity check with -fpartial-profile-training
Hi, do_estimate_edge_time tests that cached and real values matches. This test is not working precisely for global profiles because of roundoff issues when profile of clones is subtracted from profile of offline body. This is checked by presence of ipa counter. This breaks with partial profile training because we turn IPA profiles to local when they drop to 0. Bootstrapped/regtested x86_64-linux, comitted. * ipa-inline-analysis.c (do_estimate_edge_time): Silence overactive sanity check. Index: ipa-inline-analysis.c === --- ipa-inline-analysis.c (revision 279076) +++ ipa-inline-analysis.c (working copy) @@ -209,6 +209,7 @@ do_estimate_edge_time (struct cgraph_edg nonspec_time = e->entry.nonspec_time; hints = e->entry.hints; if (flag_checking + && !opt_for_fn (callee->decl, flag_profile_partial_training) && !callee->count.ipa_p ()) { sreal chk_time, chk_nonspec_time;
Fix flag_toplevel_reorder wrt optimize attribute
Hi, this is (very likely partial) fix for PR92860 where optimize attribute incorrectly rewrites flag_toplevel_reorder which we handle as global flag. In fact we support per-symbol toplevel reordering for a while so we only need to add the annotation. However we need to inspect all other flags implied by optimize that they have Optimization attribute on them. Also i think this should be backoported to release branches, but it breaks LTO streaming compaibility. Bootstrapped/regtested x86_64-linux, comitted. PR tree-optimization/92860 * common.opt (fprofile-reorder-functions, ftoplevel-reorder): Add Optimization flag. Index: common.opt === --- common.opt (revision 279076) +++ common.opt (working copy) @@ -2181,7 +2181,7 @@ Common Report Var(profile_report) Report on consistency of profile. fprofile-reorder-functions -Common Report Var(flag_profile_reorder_functions) +Common Report Var(flag_profile_reorder_functions) Optimization Enable function reordering that improves code placement. fpatchable-function-entry= @@ -2586,7 +2586,7 @@ EnumValue Enum(tls_model) String(local-exec) Value(TLS_MODEL_LOCAL_EXEC) ftoplevel-reorder -Common Report Var(flag_toplevel_reorder) Init(2) +Common Report Var(flag_toplevel_reorder) Init(2) Optimization Reorder top level functions, variables, and asms. ftracer
[patch, fortran, committed] Error on Associate with a program
Hello world, yet another obvious and simple fix: You cannot really associate the main program with a variable. Committed as r279088 after regression-testing. Regards Thomas 2018-12-08 Thomas Koenig PR fortran/92780 * resolve.c (resolve_assoc_var): Issue error if the associating entity is a program. 2018-12-08 Thomas Koenig PR fortran/92780 * gfortran.dg/associate_50.f90: New test. ! { dg-do compile } ! PR 92780 - this used to ICE instead of being rejected. ! Test case by Gerhard Steinmetz. program p associate (y => p) ! { dg-error "is a PROGRAM" } end associate end program p Index: resolve.c === --- resolve.c (Revision 279064) +++ resolve.c (Arbeitskopie) @@ -8842,6 +8842,12 @@ resolve_assoc_var (gfc_symbol* sym, bool resolve_t gcc_assert (target->symtree); tsym = target->symtree->n.sym; + if (tsym->attr.flavor == FL_PROGRAM) + { + gfc_error ("Associating entity %qs at %L is a PROGRAM", + tsym->name, &target->where); + return; + } sym->attr.asynchronous = tsym->attr.asynchronous; sym->attr.volatile_ = tsym->attr.volatile_;
[patch, fortran] Add NULL check before checking interfaces
Hello world, the attached patch fixes an ICE where a NULL check was missing. Committed as obvious and simple after regression-testing as r279087. Since this is an ICE after an error has already been emitted, I don't see any particular need to backport. Regards Thomas 2018-12-08 Thomas Koenig PR fortran/92764 * interface.c (gfc_procedure_use): Check for existence of derived component before using (twice). 2018-12-08 Thomas Koenig PR fortran/92764 * gfortran.dg/interface_44.f90: New test. Index: interface.c === --- interface.c (Revision 279064) +++ interface.c (Arbeitskopie) @@ -3888,6 +3888,7 @@ gfc_procedure_use (gfc_symbol *sym, gfc_actual_arg /* F2008, C1303 and C1304. */ if (a->expr && (a->expr->ts.type == BT_DERIVED || a->expr->ts.type == BT_CLASS) + && a->expr->ts.u.derived && ((a->expr->ts.u.derived->from_intmod == INTMOD_ISO_FORTRAN_ENV && a->expr->ts.u.derived->intmod_sym_id == ISOFORTRAN_LOCK_TYPE) || gfc_expr_attr (a->expr).lock_comp)) @@ -3901,6 +3902,7 @@ gfc_procedure_use (gfc_symbol *sym, gfc_actual_arg if (a->expr && (a->expr->ts.type == BT_DERIVED || a->expr->ts.type == BT_CLASS) + && a->expr->ts.u.derived && ((a->expr->ts.u.derived->from_intmod == INTMOD_ISO_FORTRAN_ENV && a->expr->ts.u.derived->intmod_sym_id == ISOFORTRAN_EVENT_TYPE) ! { dg-do compile } ! PR 92964 - this used to ICE. ! Original test case by Arseny Solokha type(e6) function dn() ! { dg-error "The type for function" } call sub(dn) end function dn
[patch, fortran] Fix PR 92755
Hello world, In the fix for PR 91783, I had not considered the case that the _data ref could also be somewhere inside. The solution was obvious and simple: Move the check into the loop over the refs. Committed as r279086 after regression-testing. Regards Thomas 2019-12-08 Thomas Koenig PR fortran/92755 * dependency.c (gfc_dep_resolver): Move skipping of _data ref into the loop. 2019-12-08 Thomas Koenig PR fortran/92755 * gfortran.dg/dependency_57.f90: New test. Index: dependency.c === --- dependency.c (Revision 279064) +++ dependency.c (Arbeitskopie) @@ -2098,18 +2098,6 @@ gfc_dep_resolver (gfc_ref *lref, gfc_ref *rref, gf gfc_dependency this_dep; bool same_component = false; - /* The refs might come in mixed, one with a _data component and one - without. Look at their next reference in order to avoid an - ICE. */ - - if (lref && lref->type == REF_COMPONENT && lref->u.c.component - && strcmp (lref->u.c.component->name, "_data") == 0) -lref = lref->next; - - if (rref && rref->type == REF_COMPONENT && rref->u.c.component - && strcmp (rref->u.c.component->name, "_data") == 0) -rref = rref->next; - this_dep = GFC_DEP_ERROR; fin_dep = GFC_DEP_ERROR; /* Dependencies due to pointers should already have been identified. @@ -2117,6 +2105,18 @@ gfc_dep_resolver (gfc_ref *lref, gfc_ref *rref, gf while (lref && rref) { + /* The refs might come in mixed, one with a _data component and one + without. Look at their next reference in order to avoid an + ICE. */ + + if (lref && lref->type == REF_COMPONENT && lref->u.c.component + && strcmp (lref->u.c.component->name, "_data") == 0) + lref = lref->next; + + if (rref && rref->type == REF_COMPONENT && rref->u.c.component + && strcmp (rref->u.c.component->name, "_data") == 0) + rref = rref->next; + /* We're resolving from the same base symbol, so both refs should be the same type. We traverse the reference chain until we find ranges that are not equal. */ ! { dg-do compile } ! PR 92755 - this used to cause an ICE. ! Original test case by Gerhard Steinmetz program p type t end type type t2 class(t), allocatable :: a(:) end type type(t2) :: z z%a = [z%a] end