Re: [PATCH 0/6] Parallelize Intra-Procedural Optimizations using the LTO Engine.
On Sat, Aug 22, 2020 at 06:04:48PM -0300, Giuliano Belinassi wrote: > Hi, Josh > > On 08/21, Josh Triplett wrote: > > On Thu, Aug 20, 2020 at 07:00:13PM -0300, Giuliano Belinassi wrote: > > > This patch series add a new flag "-fparallel-jobs=" to control if the > > > compiler should try to compile the current file in parallel. > > [...] > > > Bootstrapped and Regtested on Linux x86_64. > > > > > > Giuliano Belinassi (6): > > > Modify gcc driver for parallel compilation > > > Implement a new partitioner for parallel compilation > > > Implement fork-based parallelism engine > > > Add `+' for Jobserver Integration > > > Add invoke documentation > > > New tests for parallel compilation feature > > > > Very nice! > > Thank you for your interest in this :) > > > > > I'm interested in testing this on a highly parallel system. What > > baseline do these patches apply to? They don't seem to apply to GCC > > trunk. > > Hummm, this was supposed to work on trunk out of the box. However, > there is a high probability that I messed up something while rebasing. > I will post a version 2 of it when I get more comments and when I fix > the Makefile issue that Joseph pointed out in other e-mail. > > If you want to test it on a high parallel system, I think it will be > cool to see how it behaves also when --param=promote-statics=1, as it > increases parallelism opportunity. :) I plan to try several variations, including that. I'd like to see how it affects the performance of Linux kernel builds. > > Also, I tried to bootstrap the current tip of the devel/autopar_devel > > branch, but ended up with compiler segfaults that all look like this: > > ../../gcc/zlib/compress.c:86:1: internal compiler error: Segmentation fault > >86 | } > > | ^ > > Well, there was once a bug in this branch when compiling with -flto that > caused the assembler output file not to be properly initialized early > enough, resulting in LTO LGEN stage writing into a invalid FILE pointer. > I fixed this during rebasing but I forgot to push to the autopar_devel > branch. In any case, I just pushed the recent changes to autopar_devel > which fix this issue. That might explain the problem; I had tried to build gcc with the bootstrap-lto configuration. > In any case, -fparallel-jobs= should NOT be used together with -flto. > Although I used part of the LTO engine for development of this feature, > they are meant for distinct things. I guess I should give a warning > about that in next version :) Interesting. Is that something that could change in the future? I'd like to be able to get some parallelism when creating the object files, and then more parallelism when doing the final LTO link. > Also, I just tested bootstrap with > > ../gcc/configure --disable-multilib --enable-languages=c,c++ > > on x86_64 linux and it is working. I'd used --enable-multilib, and --enable-languages=c,c++,lto . Would that be expected to work? Thanks, Josh
Re: [PATCH 0/6] Parallelize Intra-Procedural Optimizations using the LTO Engine.
On Thu, Aug 20, 2020 at 07:00:13PM -0300, Giuliano Belinassi wrote: > This patch series add a new flag "-fparallel-jobs=" to control if the > compiler should try to compile the current file in parallel. [...] > Bootstrapped and Regtested on Linux x86_64. > > Giuliano Belinassi (6): > Modify gcc driver for parallel compilation > Implement a new partitioner for parallel compilation > Implement fork-based parallelism engine > Add `+' for Jobserver Integration > Add invoke documentation > New tests for parallel compilation feature Very nice! I'm interested in testing this on a highly parallel system. What baseline do these patches apply to? They don't seem to apply to GCC trunk. Also, I tried to bootstrap the current tip of the devel/autopar_devel branch, but ended up with compiler segfaults that all look like this: ../../gcc/zlib/compress.c:86:1: internal compiler error: Segmentation fault 86 | } | ^
Re: [PATCH] Refactor handle_section_attribute to reduce nesting and distinguish error cases
On Tue, Apr 28, 2015 at 06:10:33PM -0600, Jeff Law wrote: > On 04/26/2015 07:09 PM, Josh Triplett wrote: > >handle_section_attribute contains many levels of nested conditionals and > >branching code flow paths, with the error cases sometimes in the else > >case and sometimes in the if case. Simplify the code flow into a series > >of potential failure cases ending with the successful path, with no > >nesting and no else clauses. > >--- > > > >I do not have commit access; if you approve, please commit. > > > >This cleanup will help enable a larger patch enhancing the section attribute. > > > >gcc/Changelog entry: > > > >2014-08-24 Josh Triplett > > > > * c-family/c-common.c (handle_section_attribute): Refactor to reduce > > nesting and distinguish between error cases. > Have you done a bootstrap and regression test with this patch? While I > don't expect problems, it's part of the standard procedure for patch > submissions. > > There may be tests which look for "section_attribute note allowed for ..." > in the !STRING_CST case that will have a different error message after your > change. If so, they will need to be adjusted. > > I'll approve and commit once you've verified the bootstrap & regression test > was OK. Yes, I've done a full bootstrap and regression test without issue. There don't appear to be any tests that rely on the text of any of those error messages. - Josh Triplett
[PATCH] Refactor handle_section_attribute to reduce nesting and distinguish error cases
handle_section_attribute contains many levels of nested conditionals and
branching code flow paths, with the error cases sometimes in the else
case and sometimes in the if case. Simplify the code flow into a series
of potential failure cases ending with the successful path, with no
nesting and no else clauses.
---
I do not have commit access; if you approve, please commit.
This cleanup will help enable a larger patch enhancing the section attribute.
gcc/Changelog entry:
2014-08-24 Josh Triplett
* c-family/c-common.c (handle_section_attribute): Refactor to reduce
nesting and distinguish between error cases.
gcc/c-family/c-common.c | 91 +
1 file changed, 46 insertions(+), 45 deletions(-)
diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 9797e17..588953d 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -7598,58 +7598,59 @@ handle_section_attribute (tree *node, tree ARG_UNUSED
(name), tree args,
{
tree decl = *node;
- if (targetm_common.have_named_sections)
+ if (!targetm_common.have_named_sections)
{
- user_defined_section_attribute = true;
+ error_at (DECL_SOURCE_LOCATION (*node),
+ "section attributes are not supported for this target");
+ goto fail;
+}
- if ((TREE_CODE (decl) == FUNCTION_DECL
- || TREE_CODE (decl) == VAR_DECL)
- && TREE_CODE (TREE_VALUE (args)) == STRING_CST)
- {
- if (TREE_CODE (decl) == VAR_DECL
- && current_function_decl != NULL_TREE
- && !TREE_STATIC (decl))
- {
- error_at (DECL_SOURCE_LOCATION (decl),
- "section attribute cannot be specified for "
- "local variables");
- *no_add_attrs = true;
- }
+ user_defined_section_attribute = true;
- /* The decl may have already been given a section attribute
-from a previous declaration. Ensure they match. */
- else if (DECL_SECTION_NAME (decl) != NULL
- && strcmp (DECL_SECTION_NAME (decl),
- TREE_STRING_POINTER (TREE_VALUE (args))) != 0)
- {
- error ("section of %q+D conflicts with previous declaration",
-*node);
- *no_add_attrs = true;
- }
- else if (TREE_CODE (decl) == VAR_DECL
- && !targetm.have_tls && targetm.emutls.tmpl_section
- && DECL_THREAD_LOCAL_P (decl))
- {
- error ("section of %q+D cannot be overridden", *node);
- *no_add_attrs = true;
- }
- else
- set_decl_section_name (decl,
- TREE_STRING_POINTER (TREE_VALUE (args)));
- }
- else
- {
- error ("section attribute not allowed for %q+D", *node);
- *no_add_attrs = true;
- }
+ if (TREE_CODE (decl) != FUNCTION_DECL && TREE_CODE (decl) != VAR_DECL)
+{
+ error ("section attribute not allowed for %q+D", *node);
+ goto fail;
}
- else
+
+ if (TREE_CODE (TREE_VALUE (args)) != STRING_CST)
{
- error_at (DECL_SOURCE_LOCATION (*node),
- "section attributes are not supported for this target");
- *no_add_attrs = true;
+ error ("section attribute argument not a string constant");
+ goto fail;
+}
+
+ if (TREE_CODE (decl) == VAR_DECL
+ && current_function_decl != NULL_TREE
+ && !TREE_STATIC (decl))
+{
+ error_at (DECL_SOURCE_LOCATION (decl),
+"section attribute cannot be specified for local variables");
+ goto fail;
}
+ /* The decl may have already been given a section attribute
+ from a previous declaration. Ensure they match. */
+ if (DECL_SECTION_NAME (decl) != NULL
+ && strcmp (DECL_SECTION_NAME (decl),
+ TREE_STRING_POINTER (TREE_VALUE (args))) != 0)
+{
+ error ("section of %q+D conflicts with previous declaration", *node);
+ goto fail;
+}
+
+ if (TREE_CODE (decl) == VAR_DECL
+ && !targetm.have_tls && targetm.emutls.tmpl_section
+ && DECL_THREAD_LOCAL_P (decl))
+{
+ error ("section of %q+D cannot be overridden", *node);
+ goto fail;
+}
+
+ set_decl_section_name (decl, TREE_STRING_POINTER (TREE_VALUE (args)));
+ return NULL_TREE;
+
+fail:
+ *no_add_attrs = true;
return NULL_TREE;
}
--
2.1.4
Re: [PATCH] Refactor handle_section_attribute to reduce nesting and distinguish error cases
On Sun, Aug 24, 2014 at 05:47:23PM -0400, Trevor Saunders wrote:
> On Sun, Aug 24, 2014 at 01:58:52PM -0700, Andrew Pinski wrote:
> > On Sun, Aug 24, 2014 at 1:42 PM, Josh Triplett
> > wrote:
> > > handle_section_attribute contains many levels of nested conditionals and
> > > branching code flow paths, with the error cases sometimes in the else
> > > case and sometimes in the if case. Simplify the code flow into a series
> > > of potential failure cases ending with the successful path, with no
> > > nesting and no else clauses.
> > > ---
> > >
> > > I started to work on an extension to the section attribute, and found
> > > myself cleaning up the logic in handle_section_attribute; I wanted to
> > > send this cleanup patch separately.
> > >
> > > I'm not a GCC committer, so I'm looking both for review and for someone
> > > to commit this patch.
> > >
> > > gcc/ChangeLog | 5 +++
> > > gcc/c-family/c-common.c | 91
> > > +
> > > 2 files changed, 51 insertions(+), 45 deletions(-)
> > >
> > > diff --git a/gcc/ChangeLog b/gcc/ChangeLog
> > > index 703a489..0286bfb 100644
> > > --- a/gcc/ChangeLog
> > > +++ b/gcc/ChangeLog
> > > @@ -1,3 +1,8 @@
> > > +2014-08-24 Josh Triplett
> > > +
> > > + * c-family/c-common.c (handle_section_attribute): Refactor to
> > > reduce
> > > + nesting and distinguish between error cases.
> > > +
> > > 2014-08-24 Oleg Endo
> > >
> > > PR target/61996
> > > diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
> > > index 58b9763..a63eedf 100644
> > > --- a/gcc/c-family/c-common.c
> > > +++ b/gcc/c-family/c-common.c
> > > @@ -7387,58 +7387,59 @@ handle_section_attribute (tree *node, tree
> > > ARG_UNUSED (name), tree args,
> > > {
> > >tree decl = *node;
> > >
> > > - if (targetm_common.have_named_sections)
> > > + if (!targetm_common.have_named_sections)
> > > {
> > > - user_defined_section_attribute = true;
> > > + error_at (DECL_SOURCE_LOCATION (*node),
> > > + "section attributes are not supported for this target");
> > > + goto fail;
> >
> > Why not move this to a different function and then do:
> >
> > if (function(...))
> > set_decl_section_name (decl, TREE_STRING_POINTER (TREE_VALUE (args)));
> > else
> > *no_add_attrs = true;
> >
> > return NULL_TREE;
> >
> > This is a way to get away from using gotos.
>
> or you could just set no_add_attrs to true at the start of the function
> and only set it to false when you know the attr is valid, which saves
> the weirdness of a second function.
Though that would then duplicate the "return NULL_TREE" in many places,
which seems suboptimal (and not much different than "goto fail").
Andrew, any preference between those two approaches?
- Josh Triplett
> Trev
>
> >
> > Thanks,
> > Andrew
> >
> >
> > > +}
> > >
> > > - if ((TREE_CODE (decl) == FUNCTION_DECL
> > > - || TREE_CODE (decl) == VAR_DECL)
> > > - && TREE_CODE (TREE_VALUE (args)) == STRING_CST)
> > > - {
> > > - if (TREE_CODE (decl) == VAR_DECL
> > > - && current_function_decl != NULL_TREE
> > > - && !TREE_STATIC (decl))
> > > - {
> > > - error_at (DECL_SOURCE_LOCATION (decl),
> > > - "section attribute cannot be specified for "
> > > - "local variables");
> > > - *no_add_attrs = true;
> > > - }
> > > + user_defined_section_attribute = true;
> > >
> > > - /* The decl may have already been given a section attribute
> > > -from a previous declaration. Ensure they match. */
> > > - else if (DECL_SECTION_NAME (decl) != NULL
> > > - && strcmp (DECL_SECTION_NAME (decl),
> > > - TREE_STRING_POINTER (TREE_VALUE (args))) !=
> > > 0)
> > > - {
> > > - error ("section of %q+D conflicts with previous
> > > declaration",
> > > -*node);
> > > - *no_add_at
Re: [PATCH] Refactor handle_section_attribute to reduce nesting and distinguish error cases
On Sun, Aug 24, 2014 at 01:58:52PM -0700, Andrew Pinski wrote:
> On Sun, Aug 24, 2014 at 1:42 PM, Josh Triplett wrote:
> > handle_section_attribute contains many levels of nested conditionals and
> > branching code flow paths, with the error cases sometimes in the else
> > case and sometimes in the if case. Simplify the code flow into a series
> > of potential failure cases ending with the successful path, with no
> > nesting and no else clauses.
> > ---
> >
> > I started to work on an extension to the section attribute, and found
> > myself cleaning up the logic in handle_section_attribute; I wanted to
> > send this cleanup patch separately.
> >
> > I'm not a GCC committer, so I'm looking both for review and for someone
> > to commit this patch.
> >
> > gcc/ChangeLog | 5 +++
> > gcc/c-family/c-common.c | 91
> > +
> > 2 files changed, 51 insertions(+), 45 deletions(-)
> >
> > diff --git a/gcc/ChangeLog b/gcc/ChangeLog
> > index 703a489..0286bfb 100644
> > --- a/gcc/ChangeLog
> > +++ b/gcc/ChangeLog
> > @@ -1,3 +1,8 @@
> > +2014-08-24 Josh Triplett
> > +
> > + * c-family/c-common.c (handle_section_attribute): Refactor to reduce
> > + nesting and distinguish between error cases.
> > +
> > 2014-08-24 Oleg Endo
> >
> > PR target/61996
> > diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
> > index 58b9763..a63eedf 100644
> > --- a/gcc/c-family/c-common.c
> > +++ b/gcc/c-family/c-common.c
> > @@ -7387,58 +7387,59 @@ handle_section_attribute (tree *node, tree
> > ARG_UNUSED (name), tree args,
> > {
> >tree decl = *node;
> >
> > - if (targetm_common.have_named_sections)
> > + if (!targetm_common.have_named_sections)
> > {
> > - user_defined_section_attribute = true;
> > + error_at (DECL_SOURCE_LOCATION (*node),
> > + "section attributes are not supported for this target");
> > + goto fail;
>
> Why not move this to a different function and then do:
>
> if (function(...))
> set_decl_section_name (decl, TREE_STRING_POINTER (TREE_VALUE (args)));
> else
> *no_add_attrs = true;
>
> return NULL_TREE;
>
> This is a way to get away from using gotos.
I can certainly do so; I hadn't in this patch because the file in
question already contains dozens of instances of the same goto-based
error-handling pattern.
I'll send a second version of this patch with a separate function
returning bool.
- Josh Triplett
[PATCH] Refactor handle_section_attribute to reduce nesting and distinguish error cases
handle_section_attribute contains many levels of nested conditionals and
branching code flow paths, with the error cases sometimes in the else
case and sometimes in the if case. Simplify the code flow into a series
of potential failure cases ending with the successful path, with no
nesting and no else clauses.
---
I started to work on an extension to the section attribute, and found
myself cleaning up the logic in handle_section_attribute; I wanted to
send this cleanup patch separately.
I'm not a GCC committer, so I'm looking both for review and for someone
to commit this patch.
gcc/ChangeLog | 5 +++
gcc/c-family/c-common.c | 91 +
2 files changed, 51 insertions(+), 45 deletions(-)
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 703a489..0286bfb 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,8 @@
+2014-08-24 Josh Triplett
+
+ * c-family/c-common.c (handle_section_attribute): Refactor to reduce
+ nesting and distinguish between error cases.
+
2014-08-24 Oleg Endo
PR target/61996
diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 58b9763..a63eedf 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -7387,58 +7387,59 @@ handle_section_attribute (tree *node, tree ARG_UNUSED
(name), tree args,
{
tree decl = *node;
- if (targetm_common.have_named_sections)
+ if (!targetm_common.have_named_sections)
{
- user_defined_section_attribute = true;
+ error_at (DECL_SOURCE_LOCATION (*node),
+ "section attributes are not supported for this target");
+ goto fail;
+}
- if ((TREE_CODE (decl) == FUNCTION_DECL
- || TREE_CODE (decl) == VAR_DECL)
- && TREE_CODE (TREE_VALUE (args)) == STRING_CST)
- {
- if (TREE_CODE (decl) == VAR_DECL
- && current_function_decl != NULL_TREE
- && !TREE_STATIC (decl))
- {
- error_at (DECL_SOURCE_LOCATION (decl),
- "section attribute cannot be specified for "
- "local variables");
- *no_add_attrs = true;
- }
+ user_defined_section_attribute = true;
- /* The decl may have already been given a section attribute
-from a previous declaration. Ensure they match. */
- else if (DECL_SECTION_NAME (decl) != NULL
- && strcmp (DECL_SECTION_NAME (decl),
- TREE_STRING_POINTER (TREE_VALUE (args))) != 0)
- {
- error ("section of %q+D conflicts with previous declaration",
-*node);
- *no_add_attrs = true;
- }
- else if (TREE_CODE (decl) == VAR_DECL
- && !targetm.have_tls && targetm.emutls.tmpl_section
- && DECL_THREAD_LOCAL_P (decl))
- {
- error ("section of %q+D cannot be overridden", *node);
- *no_add_attrs = true;
- }
- else
- set_decl_section_name (decl,
- TREE_STRING_POINTER (TREE_VALUE (args)));
- }
- else
- {
- error ("section attribute not allowed for %q+D", *node);
- *no_add_attrs = true;
- }
+ if (TREE_CODE (decl) != FUNCTION_DECL && TREE_CODE (decl) != VAR_DECL)
+{
+ error ("section attribute not allowed for %q+D", *node);
+ goto fail;
}
- else
+
+ if (TREE_CODE (TREE_VALUE (args)) != STRING_CST)
{
- error_at (DECL_SOURCE_LOCATION (*node),
- "section attributes are not supported for this target");
- *no_add_attrs = true;
+ error ("section attribute argument not a string constant");
+ goto fail;
+}
+
+ if (TREE_CODE (decl) == VAR_DECL
+ && current_function_decl != NULL_TREE
+ && !TREE_STATIC (decl))
+{
+ error_at (DECL_SOURCE_LOCATION (decl),
+"section attribute cannot be specified for local variables");
+ goto fail;
}
+ /* The decl may have already been given a section attribute
+ from a previous declaration. Ensure they match. */
+ if (DECL_SECTION_NAME (decl) != NULL
+ && strcmp (DECL_SECTION_NAME (decl),
+ TREE_STRING_POINTER (TREE_VALUE (args))) != 0)
+{
+ error ("section of %q+D conflicts with previous declaration", *node);
+ goto fail;
+}
+
+ if (TREE_CODE (decl) == VAR_DECL
+ && !targetm.have_tls && targetm.emutls.tmpl_section
+ && DECL_THREAD_LOCAL_P (decl))
+{
+ error ("section of %q+D cannot be overridden", *node);
+ goto fail;
+}
+
+ set_decl_section_name (decl, TREE_STRING_POINTER (TREE_VALUE (args)));
+ return NULL_TREE;
+
+fail:
+ *no_add_attrs = true;
return NULL_TREE;
}
--
2.1.0
[PATCH] output.h (get_named_text_section): Fix typo in comment: s/SECITON/SECTION/
--- ChangeLog entry: 2013-11-24 Josh Triplett * output.h (get_named_text_section): Fix typo in comment: s/SECITON/SECTION/ gcc/output.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gcc/output.h b/gcc/output.h index 7b26256..3a86e1b 100644 --- a/gcc/output.h +++ b/gcc/output.h @@ -259,7 +259,7 @@ extern bool default_assemble_integer (rtx, unsigned int, int); be outputable. */ extern bool assemble_integer (rtx, unsigned, unsigned, int); -/* Return section for TEXT_SECITON_NAME if DECL or DECL_SECTION_NAME (DECL) +/* Return section for TEXT_SECTION_NAME if DECL or DECL_SECTION_NAME (DECL) is NULL. */ extern section *get_named_text_section (tree, const char *, const char *); -- 1.8.4.4
