Re: gcc/DATESTAMP not updated any longer
On 11/9/20 11:09 PM, Jakub Jelinek wrote:
So I think either new_update must be raising an exception, or returning
None for some reason.
Bet we want to add logging for that exception, as well as for it returning
None and inside of it perhaps even more detailed logging on where it
returned None.
The following should expand both a returned None or an exception.
Jakub, please install it.
Thanks,
Martin
>From c3fb1dbededb037875428ced2b66ff40dc45c9d1 Mon Sep 17 00:00:00 2001
From: marxin
Date: Tue, 10 Nov 2020 09:22:28 +0100
Subject: [PATCH] another extra logging
---
.../src-home/git-hooks/hooks/post_receive.py | 20 +--
.../git-hooks/hooks/updates/factory.py| 2 ++
2 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/sourceware/projects/src-home/git-hooks/hooks/post_receive.py b/sourceware/projects/src-home/git-hooks/hooks/post_receive.py
index 37166e6..9386e2f 100644
--- a/sourceware/projects/src-home/git-hooks/hooks/post_receive.py
+++ b/sourceware/projects/src-home/git-hooks/hooks/post_receive.py
@@ -39,17 +39,17 @@ def post_receive_one(ref_name, old_rev, new_rev, refs, submitter_email):
with open("/tmp/gcc-git-mail.log", "a") as f:
ts = str(datetime.datetime.now())
f.write('post_receive_one:%s: ref_name=%s, old_rev=%s, new_rev=%s\n' % (ts, ref_name, old_rev, new_rev))
-update = new_update(ref_name, old_rev, new_rev, refs, submitter_email)
-if update is None:
-# We emit a warning, rather than trigger an assertion, because
-# it gives the script a chance to process any other reference
-# that was updated, but not processed yet.
-warn("post-receive: Unsupported reference update: %s (ignored)."
- % ref_name,
- " old_rev = %s" % old_rev,
- " new_rev = %s" % new_rev)
-return
try:
+update = new_update(ref_name, old_rev, new_rev, refs, submitter_email)
+if update is None:
+# We emit a warning, rather than trigger an assertion, because
+# it gives the script a chance to process any other reference
+# that was updated, but not processed yet.
+warn("post-receive: Unsupported reference update: %s (ignored)."
+ % ref_name,
+ " old_rev = %s" % old_rev,
+ " new_rev = %s" % new_rev)
+return
update.send_email_notifications()
except Exception as e:
with open("/tmp/gcc-git-mail.log", "a") as f:
diff --git a/sourceware/projects/src-home/git-hooks/hooks/updates/factory.py b/sourceware/projects/src-home/git-hooks/hooks/updates/factory.py
index 663a440..42cf9cc 100644
--- a/sourceware/projects/src-home/git-hooks/hooks/updates/factory.py
+++ b/sourceware/projects/src-home/git-hooks/hooks/updates/factory.py
@@ -207,6 +207,8 @@ def new_update(ref_name, old_rev, new_rev, all_refs, submitter_email):
raise_unrecognized_ref_name(ref_name)
new_cls = REF_CHANGE_MAP.get((ref_kind, change_type, object_type), None)
+with open("/tmp/gcc-git-mail.log", "a") as f:
+f.write('REF_CHANGE_MAP: ref_kind=%s, change_type=%s, object_type=%s, new_cls=%s' % (str(ref_kind), str(change_type), str(object_type), str(new_cls)))
if new_cls is None:
return None
--
2.29.2
Re: Detect EAF flags in ipa-modref
> > Alias oracle query stats: > refs_may_alias_p: 65808750 disambiguations, 75664890 queries > ref_maybe_used_by_call_p: 153485 disambiguations, 66711204 queries > call_may_clobber_ref_p: 22816 disambiguations, 28889 queries > nonoverlapping_component_refs_p: 0 disambiguations, 36846 queries > nonoverlapping_refs_since_match_p: 27271 disambiguations, 58917 must > overlaps, 86958 queries > aliasing_component_refs_p: 65808 disambiguations, 2067256 queries > TBAA oracle: 25929211 disambiguations 60395141 queries >12391384 are in alias set 0 >10783783 queries asked about the same object >126 queries asked about the same alias set >0 access volatile >9598698 are dependent in the DAG >1691939 are aritificially in conflict with void * > > Modref stats: > modref use: 14284 disambiguations, 53336 queries > modref clobber: 1660281 disambiguations, 2130440 queries > 4311165 tbaa queries (2.023603 per modref query) > 685304 base compares (0.321673 per modref query) > > PTA query stats: > pt_solution_includes: 959190 disambiguations, 13169678 queries > pt_solutions_intersect: 1050969 disambiguations, 13246686 queries I re-run the mainline withtout EAF flag propagation and the results are correct, so it is not due to independent change pushed between my tests. Honza
Re: Detect EAF flags in ipa-modref
> Bootstrapped/regtested x86_64-linux, OK? > (I will analyze more the t2.c failure) I have found independent reproducer that is now in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97775 Honza
Re: Detect EAF flags in ipa-modref
> > Bootstrapped/regtested x86_64-linux, OK? > > (I will analyze more the t2.c failure) > > I have found independent reproducer that is now in > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97775 ... and with Jakub's fix the testcase works now. Honza > > Honza
Re: Detect EAF flags in ipa-modref
On Tue, 10 Nov 2020, Jan Hubicka wrote: > Hi, > this is updated patch for autodetection of EAF flags. Still the goal is > to avoid fancy stuff and get besic logic in place (so no dataflow, no IPA > propagation, no attempts to handle trickier cases). There is one new failure > > ./gcc/testsuite/gcc/gcc.sum:FAIL: gcc.dg/sso/t2.c -Wno-scalar-storage-order > -O1 -fno-inline output pattern test > ./gcc/testsuite/gcc/gcc.sum:FAIL: gcc.dg/sso/t2.c -Wno-scalar-storage-order > -O2 output pattern test > ./gcc/testsuite/gcc/gcc.sum:FAIL: gcc.dg/sso/t2.c -Wno-scalar-storage-order > -Os output pattern test > > Which I blieve is bug exposed by detecting dump function to be EAF_DIRECT and > NOESCAPE (which it is) and packing/updacking the bitfields leads in one bit > difference. Still no idea why. > > Patch seems to be quite effective on cc1plus turning: > > Alias oracle query stats: > refs_may_alias_p: 65808750 disambiguations, 75664890 queries > ref_maybe_used_by_call_p: 153485 disambiguations, 66711204 queries > call_may_clobber_ref_p: 22816 disambiguations, 28889 queries > nonoverlapping_component_refs_p: 0 disambiguations, 36846 queries > nonoverlapping_refs_since_match_p: 27271 disambiguations, 58917 must > overlaps, 86958 queries > aliasing_component_refs_p: 65808 disambiguations, 2067256 queries > TBAA oracle: 25929211 disambiguations 60395141 queries >12391384 are in alias set 0 >10783783 queries asked about the same object >126 queries asked about the same alias set >0 access volatile >9598698 are dependent in the DAG >1691939 are aritificially in conflict with void * > > Modref stats: > modref use: 14284 disambiguations, 53336 queries > modref clobber: 1660281 disambiguations, 2130440 queries > 4311165 tbaa queries (2.023603 per modref query) > 685304 base compares (0.321673 per modref query) > > PTA query stats: > pt_solution_includes: 959190 disambiguations, 13169678 queries > pt_solutions_intersect: 1050969 disambiguations, 13246686 queries > > Alias oracle query stats: > refs_may_alias_p: 66914578 disambiguations, 76692648 queries > ref_maybe_used_by_call_p: 244077 disambiguations, 67732086 queries > call_may_clobber_ref_p: 111475 disambiguations, 116613 queries > nonoverlapping_component_refs_p: 0 disambiguations, 37091 queries > nonoverlapping_refs_since_match_p: 27267 disambiguations, 59019 must > overlaps, 87056 queries > aliasing_component_refs_p: 65870 disambiguations, 2063394 queries > TBAA oracle: 26024415 disambiguations 60579490 queries >12450910 are in alias set 0 >10806673 queries asked about the same object >125 queries asked about the same alias set >0 access volatile >9605837 are dependent in the DAG >1691530 are aritificially in conflict with void * > > Modref stats: > modref use: 14272 disambiguations, 277680 queries > modref clobber: 1669753 disambiguations, 7849135 queries > 4330162 tbaa queries (0.551674 per modref query) > 699241 base compares (0.089085 per modref query) > > PTA query stats: > pt_solution_includes: 1833920 disambiguations, 13846032 queries > pt_solutions_intersect: 1093785 disambiguations, 13309954 queries > > So almost twice as many pt_solution_includes disambiguations. > I will re-run the stats overnight to be sure that it is not independent > change (but both build was from almost same checkout). > > Bootstrapped/regtested x86_64-linux, OK? > (I will analyze more the t2.c failure) > > Honza > > gcc/ChangeLog: > > 2020-11-10 Jan Hubicka > > * gimple.c: Include ipa-modref-tree.h and ipa-modref.h. > (gimple_call_arg_flags): Use modref to determine flags. > * ipa-modref.c: Include gimple-ssa.h, tree-phinodes.h, > tree-ssa-operands.h, stringpool.h and tree-ssanames.h. > (analyze_ssa_name_flags): Declare. > (modref_summary::useful_p): Summary is also useful if arg flags are > known. > (dump_eaf_flags): New function. > (modref_summary::dump): Use it. > (get_modref_function_summary): Be read for current_function_decl > being NULL. > (memory_access_to): New function. > (deref_flags): New function. > (call_lhs_flags): New function. > (analyze_parms): New function. > (analyze_function): Use it. > * ipa-modref.h (struct modref_summary): Add arg_flags. > > gcc/testsuite/ChangeLog: > > 2020-11-10 Jan Hubicka > > * gcc.dg/torture/pta-ptrarith-1.c: Escape parametrs. > > diff --git a/gcc/gimple.c b/gcc/gimple.c > index 1afed88e1f1..da90716aa23 100644 > --- a/gcc/gimple.c > +++ b/gcc/gimple.c > @@ -46,6 +46,8 @@ along with GCC; see the file COPYING3. If not see > #include "asan.h" > #include "langhooks.h" > #include "attr-fnspec.h" > +#include "ipa-modref-tree.h" > +#include "ipa-modref.h" >
Re: Gcov info registration without constructor?
On 11/9/20 6:45 PM, Sebastian Huber wrote: Hello, Hello. There was a similar need some time ago: https://gcc.gnu.org/legacy-ml/gcc/2019-11/msg9.html Please take a look for a possible inspiration. I would like to use the -ftest-coverage -fprofile-arcs support on a bare metal system (no operating system or very early stages in the system startup). In this environment I cannot use the gcov info registration via a constructor and __gcov_init(), because there may be some other (more complex) constructors registered which cannot be called at this stage.. Would it be acceptable to add a compiler option which changes the gcov info registration via a constructor to a linker set? If enabled, then for each translation unit (see coverage_obj_init()) a pointer to the gcov info is placed into a special linker section (for example .gcov_info). The linker script collects all .gcov_info data and adds a begin/end symbol. The runtime support can then iterate over all linker section entries (pointers to struct gcov_info) to dump the aggregated gcov data during program termination. Would such changes be acceptable for GCC integration or is this too specific? That's definitely something we can support. Similarly to the linked message, are you capable of using normal system I/O to stream .gcda files? Martin Kind regards, Sebastian
Re: Gcov info registration without constructor?
Hallo Martin, On 10/11/2020 13:05, Martin Liška wrote: On 11/9/20 6:45 PM, Sebastian Huber wrote: Hello, Hello. There was a similar need some time ago: https://gcc.gnu.org/legacy-ml/gcc/2019-11/msg9.html Please take a look for a possible inspiration. thanks for the pointer. I would like to use the -ftest-coverage -fprofile-arcs support on a bare metal system (no operating system or very early stages in the system startup). In this environment I cannot use the gcov info registration via a constructor and __gcov_init(), because there may be some other (more complex) constructors registered which cannot be called at this stage.. Would it be acceptable to add a compiler option which changes the gcov info registration via a constructor to a linker set? If enabled, then for each translation unit (see coverage_obj_init()) a pointer to the gcov info is placed into a special linker section (for example .gcov_info). The linker script collects all .gcov_info data and adds a begin/end symbol. The runtime support can then iterate over all linker section entries (pointers to struct gcov_info) to dump the aggregated gcov data during program termination. Would such changes be acceptable for GCC integration or is this too specific? That's definitely something we can support. Similarly to the linked message, are you capable of using normal system I/O to stream .gcda files? I cannot use I/O to files. I also cannot use dynamic memory (e.g. malloc()). This is actually not an issue, since it is very easy to dump the gcov info via a simple character output function as plain text. In the Linux kernel see for example convert_to_gcda() in "kernel/gcov/gcc_4_7.c". Instead of copying the data to a buffer you can directly output the data via some printk() function. On the host computer you can collect the text and generate the .gcda files. I guess the first thing we need is a name for the option. What about: -fprofile-info-section -fprofile-info-section=name Register a pointer to the profile information generated by -fprofile-arcs in the named section for each translation unit. If name is not given, then .gcov_info will be used. This option disables the profile information registration through a constructor and it disables the profile information processing through a destructor. This option can be used to support profiling in environments which do not support constructors and destructors. The linker could collect the content of the named section in a continuous memory block and add begin and end symbols. The runtime support could dump the profiling information registered in this linker set during program termination to a serial line for example. -- embedded brains GmbH Sebastian HUBER Dornierstr. 4 82178 Puchheim Germany email: sebastian.hu...@embedded-brains.de Phone: +49-89-18 94 741 - 16 Fax: +49-89-18 94 741 - 08 PGP: Public key available on request. embedded brains GmbH Registergericht: Amtsgericht München Registernummer: HRB 157899 Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler Unsere Datenschutzerklärung finden Sie hier: https://embedded-brains.de/datenschutzerklaerung/
Re: Detect EAF flags in ipa-modref
> > + tree callee = gimple_call_fndecl (stmt);
> > + if (callee)
> > +{
> > + cgraph_node *node = cgraph_node::get (callee);
> > + modref_summary *summary = node ? get_modref_function_summary (node)
> > + : NULL;
> > +
> > + if (summary && summary->arg_flags.length () > arg)
>
> So could we make modref "transform" push this as fnspec attribute or
> would that not really be an optimization?
It was my original plan to synthetize fnspecs, but I think it is not
very good idea: we have the summary readily available and we can
represent information that fnspecs can't
(do not have artificial limits on number of parameters or counts)
I would preffer fnspecs to be used only for in-compiler declarations.
> > +
> > +/* Analyze EAF flags for SSA name NAME.
> > + KNOWN_FLAGS is a cache for flags we already determined.
> > + DEPTH is a recursion depth used to make debug output prettier. */
> > +
> > +static int
> > +analyze_ssa_name_flags (tree name, vec *known_flags, int depth)
>
> C++ has references which makes the access to known_flags nicer ;)
Yay, will chang that :)
>
> > +{
> > + imm_use_iterator ui;
> > + gimple *use_stmt;
> > + int flags = EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE | EAF_UNUSED;
> > +
> > + /* See if value is already computed. */
> > + if ((*known_flags)[SSA_NAME_VERSION (name)])
> > +{
> > + /* Punt on cycles for now, so we do not need dataflow. */
> > + if ((*known_flags)[SSA_NAME_VERSION (name)] == 1)
> > + {
> > + if (dump_file)
> > + fprintf (dump_file,
> > +"%*sGiving up on a cycle in SSA graph\n", depth * 4, "");
> > + return 0;
> > + }
> > + return (*known_flags)[SSA_NAME_VERSION (name)] - 2;
> > +}
> > + /* Recursion guard. */
> > + (*known_flags)[SSA_NAME_VERSION (name)] = 1;
>
> This also guards against multiple evaluations of the same stmts
> but only in some cases? Consider
>
> _1 = ..;
> _2 = _1 + _3;
> _4 = _1 + _5;
> _6 = _2 + _4;
>
> where we visit _2 = and _4 = from _1 but from both are going
> to visit _6.
Here we first push _6, then we go for _2 then for _1 evaluate _1,
evalueate _2, go for _4 and evaluate _4, and evaluate _6.
It is DFS and you need backward edge in DFS (comming from a PHI).
Cycles seems to somewhat matter for GCC: we do have a lot of functions
that walk linked lists that we could track otherwise.
>
> Maybe I'm blind but you're not limiting depth? Guess that asks
> for problems, esp. as you are recursing rather than using a
> worklist or so?
>
> I see you try to "optimize" the walk by only visiting def->use
> links from parameters but then a RPO walk over all stmts would
> be simpler iteration-wise ...
We usually evaluate just small part of bigger functions (since we lose
track quite easily after hitting first memory store). My plan was to
change this to actual dataflow once we have it well defined
(this means after discussing EAF flags with you and adding the logic to
track callsites for true IPA pass that midly complicated things - for
every ssa name I track callsite/arg pair where it is passed to
either directly or indirectly. Then this is translaed into call summary
and used by IPA pass to compute final flags)
I guess I can add --param ipa-modref-walk-depth for now and handle
dataflow incremntally?
In particular I am not sure if I should just write iterated RPO myself
or use tree-ssa-propagate.h (the second may be overkill).
>
> > + if (dump_file)
> > +{
> > + fprintf (dump_file,
> > + "%*sAnalyzing flags of ssa name: ", depth * 4, "");
> > + print_generic_expr (dump_file, name);
> > + fprintf (dump_file, "\n");
> > +}
> > +
> > + FOR_EACH_IMM_USE_STMT (use_stmt, ui, name)
> > +{
> > + if (flags == 0)
> > + {
> > + BREAK_FROM_IMM_USE_STMT (ui);
> > + }
> > + if (is_gimple_debug (use_stmt))
> > + continue;
> > + if (dump_file)
> > + {
> > + fprintf (dump_file, "%*s Analyzing stmt:", depth * 4, "");
> > + print_gimple_stmt (dump_file, use_stmt, 0);
> > + }
> > +
> > + /* Gimple return may load the return value. */
> > + if (gimple_code (use_stmt) == GIMPLE_RETURN)
>
> if (greturn *ret = dyn_cast (use_stmt))
>
> makes the as_a below not needed, similar for the other cases (it
> also makes accesses cheaper, avoiding some checking code).
Looks cleaner indeed.
>
> > + {
> > + if (memory_access_to (gimple_return_retval
> > + (as_a (use_stmt)), name))
> > + flags &= ~EAF_UNUSED;
> > + }
> > + /* Account for LHS store, arg loads and flags from callee function.
> > */
> > + else if (is_gimple_call (use_stmt))
> > + {
> > + tree callee = gimple_call_fndecl (use_stmt);
> > +
> > + /* Recursion would require bit of propagation; give up for now. */
> > + if (callee && recursive_call_p (current_function_decl, callee))
> > + flags = 0;
> > + else
> > +
Re: Detect EAF flags in ipa-modref
Hi,
here is updaed patch.
Honza
Bootstrapped/regtested x86_64-linux, OK (after the fnspec fixes)?
2020-11-10 Jan Hubicka
* gimple.c: Include ipa-modref-tree.h and ipa-modref.h.
(gimple_call_arg_flags): Use modref to determine flags.
* ipa-modref.c: Include gimple-ssa.h, tree-phinodes.h,
tree-ssa-operands.h, stringpool.h and tree-ssanames.h.
(analyze_ssa_name_flags): Declare.
(modref_summary::useful_p): Summary is also useful if arg flags are
known.
(dump_eaf_flags): New function.
(modref_summary::dump): Use it.
(get_modref_function_summary): Be read for current_function_decl
being NULL.
(memory_access_to): New function.
(deref_flags): New function.
(call_lhs_flags): New function.
(analyze_parms): New function.
(analyze_function): Use it.
* ipa-modref.h (struct modref_summary): Add arg_flags.
* doc/invoke.texi (ipa-modref-max-depth): Document.
* params.opt (ipa-modref-max-depth): New param.
gcc/testsuite/ChangeLog:
2020-11-10 Jan Hubicka
* gcc.dg/torture/pta-ptrarith-1.c: Escape parametrs.
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index d2a188d7c75..0bd76d2841e 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -12953,6 +12953,10 @@ memory locations using the mod/ref information. This
parameter ought to be
bigger than @option{--param ipa-modref-max-bases} and @option{--param
ipa-modref-max-refs}.
+@item ipa-modref-max-depth
+Specified the maximum depth of DFS walk used by modref escape analysis.
+Setting to 0 disables the analysis completely.
+
@item profile-func-internal-id
A parameter to control whether to use function internal id in profile
database lookup. If the value is 0, the compiler uses an id that
diff --git a/gcc/gimple.c b/gcc/gimple.c
index 1afed88e1f1..da90716aa23 100644
--- a/gcc/gimple.c
+++ b/gcc/gimple.c
@@ -46,6 +46,8 @@ along with GCC; see the file COPYING3. If not see
#include "asan.h"
#include "langhooks.h"
#include "attr-fnspec.h"
+#include "ipa-modref-tree.h"
+#include "ipa-modref.h"
/* All the tuples have their operand vector (if present) at the very bottom
@@ -1532,24 +1534,45 @@ int
gimple_call_arg_flags (const gcall *stmt, unsigned arg)
{
attr_fnspec fnspec = gimple_call_fnspec (stmt);
-
- if (!fnspec.known_p ())
-return 0;
-
int flags = 0;
- if (!fnspec.arg_specified_p (arg))
-;
- else if (!fnspec.arg_used_p (arg))
-flags = EAF_UNUSED;
- else
+ if (fnspec.known_p ())
{
- if (fnspec.arg_direct_p (arg))
- flags |= EAF_DIRECT;
- if (fnspec.arg_noescape_p (arg))
- flags |= EAF_NOESCAPE;
- if (fnspec.arg_readonly_p (arg))
- flags |= EAF_NOCLOBBER;
+ if (!fnspec.arg_specified_p (arg))
+ ;
+ else if (!fnspec.arg_used_p (arg))
+ flags = EAF_UNUSED;
+ else
+ {
+ if (fnspec.arg_direct_p (arg))
+ flags |= EAF_DIRECT;
+ if (fnspec.arg_noescape_p (arg))
+ flags |= EAF_NOESCAPE;
+ if (fnspec.arg_readonly_p (arg))
+ flags |= EAF_NOCLOBBER;
+ }
+}
+ tree callee = gimple_call_fndecl (stmt);
+ if (callee)
+{
+ cgraph_node *node = cgraph_node::get (callee);
+ modref_summary *summary = node ? get_modref_function_summary (node)
+ : NULL;
+
+ if (summary && summary->arg_flags.length () > arg)
+ {
+ int modref_flags = summary->arg_flags[arg];
+
+ /* We have possibly optimized out load. Be conservative here. */
+ if (!node->binds_to_current_def_p ())
+ {
+ if ((modref_flags & EAF_UNUSED) && !(flags & EAF_UNUSED))
+ modref_flags &= ~EAF_UNUSED;
+ if ((modref_flags & EAF_DIRECT) && !(flags & EAF_DIRECT))
+ modref_flags &= ~EAF_DIRECT;
+ }
+ flags |= modref_flags;
+ }
}
return flags;
}
diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c
index 3f46bebed3c..30e76580fb0 100644
--- a/gcc/ipa-modref.c
+++ b/gcc/ipa-modref.c
@@ -61,6 +61,15 @@ along with GCC; see the file COPYING3. If not see
#include "ipa-fnsummary.h"
#include "attr-fnspec.h"
#include "symtab-clones.h"
+#include "gimple-ssa.h"
+#include "tree-phinodes.h"
+#include "tree-ssa-operands.h"
+#include "ssa-iterators.h"
+#include "stringpool.h"
+#include "tree-ssanames.h"
+
+static int analyze_ssa_name_flags (tree name,
+ vec &known_flags, int depth);
/* We record fnspec specifiers for call edges since they depends on actual
gimple statements. */
@@ -186,6 +195,8 @@ modref_summary::useful_p (int ecf_flags)
{
if (ecf_flags & (ECF_CONST | ECF_NOVOPS))
return false;
+ if (arg_flags.length ())
+return true;
if (loads && !loads->every_base)
return true;
if (ecf_flags & ECF_PURE)
@@ -355,6 +366,22 @@ dump_lto_records (modref_records_lto *tt, FIL
Re: Gcov info registration without constructor?
Hello Martin,
attached is a proof of concept. I am not sure how I can make the new
section read-only. Currently, it is writable:
.section .gcov_info,"aw"
.align 2
.type .LPBX2, @object
.size .LPBX2, 4
.LPBX2:
.long .LPBX0
I probably need also a patch for the GCC options documentation, test
cases, a GCC bootstrap on Linux, release notes, ...? Do I have to wait
for the GCC 11 development start?
--
embedded brains GmbH
Sebastian HUBER
Dornierstr. 4
82178 Puchheim
Germany
email: sebastian.hu...@embedded-brains.de
Phone: +49-89-18 94 741 - 16
Fax: +49-89-18 94 741 - 08
PGP: Public key available on request.
embedded brains GmbH
Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/
>From 305eb4066742418d3b14ee6e8bec76bfb2835a99 Mon Sep 17 00:00:00 2001
From: Sebastian Huber
Date: Tue, 10 Nov 2020 16:21:07 +0100
Subject: [PATCH] Add -fprofile-info-section support
---
gcc/common.opt | 8
gcc/coverage.c | 19 +--
gcc/opts.c | 4
3 files changed, 29 insertions(+), 2 deletions(-)
diff --git a/gcc/common.opt b/gcc/common.opt
index 7d0e0d9c88a..1b69da681e3 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -2268,6 +2268,14 @@ fprofile-generate=
Common Joined RejectNegative
Enable common options for generating profile info for profile feedback directed optimizations, and set -fprofile-dir=.
+fprofile-info-section
+Common RejectNegative
+Register a pointer to the profile information in the .gcov_info section.
+
+fprofile-info-section=
+Common Joined RejectNegative Var(profile_info_section)
+Register a pointer to the profile information in the named section.
+
fprofile-partial-training
Common Report Var(flag_profile_partial_training) Optimization
Do not assume that functions never executed during the train run are cold.
diff --git a/gcc/coverage.c b/gcc/coverage.c
index 7711412c3be..ec1c5d3d125 100644
--- a/gcc/coverage.c
+++ b/gcc/coverage.c
@@ -1151,8 +1151,23 @@ coverage_obj_init (void)
ASM_GENERATE_INTERNAL_LABEL (name_buf, "LPBX", 0);
DECL_NAME (gcov_info_var) = get_identifier (name_buf);
- build_init_ctor (gcov_info_type);
- build_gcov_exit_decl ();
+ if (profile_info_section)
+{
+ tree var = build_decl (BUILTINS_LOCATION,
+ VAR_DECL, NULL_TREE,
+ build_pointer_type (gcov_info_type));
+ TREE_STATIC (var) = 1;
+ ASM_GENERATE_INTERNAL_LABEL (name_buf, "LPBX", 2);
+ DECL_NAME (var) = get_identifier (name_buf);
+ set_decl_section_name (var, profile_info_section);
+ DECL_INITIAL (var) = build_fold_addr_expr (gcov_info_var);
+ varpool_node::finalize_decl (var);
+}
+ else
+{
+ build_init_ctor (gcov_info_type);
+ build_gcov_exit_decl ();
+}
return true;
}
diff --git a/gcc/opts.c b/gcc/opts.c
index 96291e89a49..fd6e669471e 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -2602,6 +2602,10 @@ common_handle_option (struct gcc_options *opts,
SET_OPTION_IF_UNSET (opts, opts_set, flag_ipa_bit_cp, value);
break;
+case OPT_fprofile_info_section:
+ opts->x_profile_info_section = ".gcov_info";
+ break;
+
case OPT_fpatchable_function_entry_:
{
char *patch_area_arg = xstrdup (arg);
--
2.26.2
Re: Gcov info registration without constructor?
On 10/11/2020 17:23, Sebastian Huber wrote: I am not sure how I can make the new section read-only. Currently, it is writable: .section .gcov_info,"aw" .align 2 .type .LPBX2, @object .size .LPBX2, 4 .LPBX2: .long .LPBX0 I changed the section by adding: TREE_READONLY (var) = 1; get_section (profile_info_section, SECTION_UNNAMED, NULL); This is the generated data: .section .gcov_info,"a" .align 2 .type .LPBX2, @object .size .LPBX2, 4 .LPBX2: .long .LPBX0 .section .rodata .align 2 -- embedded brains GmbH Sebastian HUBER Dornierstr. 4 82178 Puchheim Germany email: sebastian.hu...@embedded-brains.de Phone: +49-89-18 94 741 - 16 Fax: +49-89-18 94 741 - 08 PGP: Public key available on request. embedded brains GmbH Registergericht: Amtsgericht München Registernummer: HRB 157899 Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler Unsere Datenschutzerklärung finden Sie hier: https://embedded-brains.de/datenschutzerklaerung/
Re: typeof and operands in named address spaces
On Mon, Nov 9, 2020 at 11:57 PM Peter Zijlstra wrote: > > Stripping const to delcare another variable is useful though. Sure C has > sharp edges, esp. if you cast stuff, but since when did that stop anyone > ;-) > > The point is, C++ has these very nice template helpers that can strip > qualifiers, I want that too, for much of the same reasons. We might not > have templates :-(, but we've become very creative with our > pre-processor. > > Surely our __unqual_scalar_typeof() cries for a better solution. Yeah, and those macros bloat the hell out of our compile times, for both compilers. I think it's reasonable to provide variants of typeof() that strip qualifiers. Some questions to flesh out more of a design. Would we want such a feature to strip all qualifiers or just specific individual ones? The more specific variants could be composed, ie. nonconst_typeof(x) y = x + 1; nonvol_typeof(z) w = z + 1; #define nonqual_typeof(v) nonconst_typeof(nonvol_typeof(v)) nonqual_typeof(v) k = v + 1; vs just: nonqual_typeof(v) k = v + 1; When I think of qualifiers, I think of const and volatile. I'm not sure why the first post I'm cc'ed on talks about "segment" qualifiers. Maybe it's in reference to a variable attribute that the kernel defines? Looking at Clang's Qualifier class, I see const, volatile, restrict (ah, right), some Objective-C stuff, and address space (TR18037 is referenced, I haven't looked up what that is) though maybe "segment" pseudo qualifiers the kernel defines expand to address space variable attributes? Maybe stripping all qualifiers is fine since you can add them back in if necessary? const volatile foo; const nonqual_typeof(foo) bar = foo; // strips off both qualifiers, re-adds const to bar -- Thanks, ~Nick Desaulniers
Re: typeof and operands in named address spaces
On Tue, Nov 10, 2020 at 10:42:58AM -0800, Nick Desaulniers wrote:
> When I think of qualifiers, I think of const and volatile. I'm not
> sure why the first post I'm cc'ed on talks about "segment" qualifiers.
> Maybe it's in reference to a variable attribute that the kernel
> defines? Looking at Clang's Qualifier class, I see const, volatile,
> restrict (ah, right), some Objective-C stuff, and address space
> (TR18037 is referenced, I haven't looked up what that is) though maybe
> "segment" pseudo qualifiers the kernel defines expand to address space
> variable attributes?
Right, x86 Named Address Space:
https://gcc.gnu.org/onlinedocs/gcc-10.2.0/gcc/Named-Address-Spaces.html#Named-Address-Spaces
Also, Google found me this:
https://reviews.llvm.org/D64676
The basic problem seems to be they act exactly like qualifiers in that
typeof() preserves them, so if you have:
( and now I realize the parent isn't Cc'd to LKML, find here:
https://gcc.gnu.org/pipermail/gcc/2020-November/234119.html )
> --cut here--
> #define foo(_var)\
> ({\
> typeof(_var) tmp__;\
> asm ("mov %1, %0" : "=r"(tmp__) : "m"(_var));\
> tmp__;\
> })
>
> __seg_fs int x;
>
> int test (void)
> {
> int y;
>
> y = foo (x);
> return y;
> }
> --cut here--
> when compiled with -O2 for x86 target, the compiler reports:
>
> pcpu.c: In function ‘test’:
> pcpu.c:14:3: error: ‘__seg_fs’ specified for auto variable ‘tmp__’
> Maybe stripping all qualifiers is fine since you can add them back in
> if necessary?
So far that seems sufficient. Although the Devil's advocate in me is
trying to construct a case where we need to preserve const but strip
volatile and that's then means we need to detect if the original has
const or not, because unconditionally adding it will be wrong.
