Re: Passes are now C++ classes (was Re: [PATCH 03/11] Handwritten part of conversion of passes to C++ classes)
> The mips*-*-* targets are not building. It looks like the mips reorg > pass (pass_mips_machine_reorg2) in config/mips/mips.c was not converted > and/or was not converted correctly. Likewise for the SPARC targets, because of the same issue. David, could you take a quick look? Thanks in advance. -- Eric Botcazou
Re: Passes are now C++ classes (was Re: [PATCH 03/11] Handwritten part of conversion of passes to C++ classes)
On Mon, 2013-08-05 at 17:03 -0400, David Malcolm wrote:
> On Mon, 2013-07-29 at 15:41 -0600, Jeff Law wrote:
> > On 07/26/2013 09:04 AM, David Malcolm wrote:
> > > This patch is the hand-written part of the conversion of passes from
> > > C structs to C++ classes. It does not work without the subsequent
> > > autogenerated part, which is huge.
> > [ ... ]
> > With the changes from pipeline -> pass_manager this is fine. As is the
> > follow-up autogenerated patch.
>
> I've committed this and the other patches that needed to go with it to
> trunk, with the name fix, having successfully bootstrapped and tested
> them on x86_64-unknown-linux-gnu - so opt_pass is now a C++ class
> hierarchy, allowing for pass instances to hold pass-specific data
> (albeit not GTY refs yet),
Did you follow my suggestion of putting __FILE__ and __LINE__ in
instances of opt_pass? I really hope that will go into 4.9!
Cheers
--
Basile STARYNKEVITCH http://starynkevitch.net/Basile/
email: basilestarynkevitchnet mobile: +33 6 8501 2359
8, rue de la Faiencerie, 92340 Bourg La Reine, France
*** opinions {are only mine, sont seulement les miennes} ***
Re: [Patch] Unbreak build on mips (was Re: Passes are now C++ classes (was Re: [PATCH 03/11] Handwritten part of conversion of passes to C++ classes))
On Tue, 2013-08-06 at 19:11 +0100, Richard Sandiford wrote: > David Malcolm writes: > > commit 11d46884e8bd9802b0f528a16b3970b4076fe8a9 > > Author: David Malcolm > > Date: Tue Aug 6 13:48:59 2013 -0400 > > > > gcc/ > > * config/mips/mips.c (insert_pass_mips_machine_reorg2): Move > > into... > > (mips_option_override): ...here, porting to new C++ API for > > passes. > > OK. Thanks for the quick fix! Thanks; committed to trunk as r201542.
Re: [Patch] Unbreak build on mips (was Re: Passes are now C++ classes (was Re: [PATCH 03/11] Handwritten part of conversion of passes to C++ classes))
David Malcolm writes: > commit 11d46884e8bd9802b0f528a16b3970b4076fe8a9 > Author: David Malcolm > Date: Tue Aug 6 13:48:59 2013 -0400 > > gcc/ > * config/mips/mips.c (insert_pass_mips_machine_reorg2): Move > into... > (mips_option_override): ...here, porting to new C++ API for > passes. OK. Thanks for the quick fix! Richard
[Patch] Unbreak build on mips (was Re: Passes are now C++ classes (was Re: [PATCH 03/11] Handwritten part of conversion of passes to C++ classes))
On Tue, 2013-08-06 at 08:16 -0700, Steve Ellcey wrote:
> On Mon, 2013-08-05 at 17:03 -0400, David Malcolm wrote:
>
> > Given all of the above testing I'm reasonably confident that this works.
> > However this is such a large change [1] that there's a non-zero chance
> > of at least one glitch - let me know if you see any breakages.
>
> The mips*-*-* targets are not building. It looks like the mips reorg
> pass (pass_mips_machine_reorg2) in config/mips/mips.c was not converted
> and/or was not converted correctly.
Sorry about this.
I was able to reproduce this build error with configure
--target=mips-linux-elf:
../../src/gcc/config/mips/mips.c:16379:28: error: expected
primary-expression before ‘.’ token
I'm attaching a patch which fixes that issue for me. Only lightly
tested (build&host=x86_64, target as above) - I'm able to build stage1,
and cc1 appears to generate assembler on a simple test case. I stepped
through the changed code in the debugger and it appears to do the right
thing. However I'm not familiar with the internals of the pass in
question.
commit 11d46884e8bd9802b0f528a16b3970b4076fe8a9
Author: David Malcolm
Date: Tue Aug 6 13:48:59 2013 -0400
gcc/
* config/mips/mips.c (insert_pass_mips_machine_reorg2): Move
into...
(mips_option_override): ...here, porting to new C++ API for
passes.
diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index 05ba003..4da80f4 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -56,6 +56,7 @@ along with GCC; see the file COPYING3. If not see
#include "target-globals.h"
#include "opts.h"
#include "tree-pass.h"
+#include "context.h"
/* True if X is an UNSPEC wrapper around a SYMBOL_REF or LABEL_REF. */
#define UNSPEC_ADDRESS_P(X) \
@@ -16374,13 +16375,6 @@ make_pass_mips_machine_reorg2 (gcc::context *ctxt)
return new pass_mips_machine_reorg2 (ctxt);
}
-struct register_pass_info insert_pass_mips_machine_reorg2 =
-{
- &pass_mips_machine_reorg2.pass, /* pass */
- "dbr",/* reference_pass_name */
- 1, /* ref_pass_instance_number */
- PASS_POS_INSERT_AFTER /* po_op */
-};
/* Implement TARGET_ASM_OUTPUT_MI_THUNK. Generate rtl rather than asm text
in order to avoid duplicating too much logic from elsewhere. */
@@ -17174,6 +17168,14 @@ mips_option_override (void)
/* We register a second machine specific reorg pass after delay slot
filling. Registering the pass must be done at start up. It's
convenient to do it here. */
+ opt_pass *new_pass = make_pass_mips_machine_reorg2 (g);
+ struct register_pass_info insert_pass_mips_machine_reorg2 =
+{
+ new_pass, /* pass */
+ "dbr", /* reference_pass_name */
+ 1, /* ref_pass_instance_number */
+ PASS_POS_INSERT_AFTER /* po_op */
+};
register_pass (&insert_pass_mips_machine_reorg2);
if (TARGET_HARD_FLOAT_ABI && TARGET_MIPS5900)
Re: Passes are now C++ classes (was Re: [PATCH 03/11] Handwritten part of conversion of passes to C++ classes)
On Mon, 2013-08-05 at 17:03 -0400, David Malcolm wrote: > Given all of the above testing I'm reasonably confident that this works. > However this is such a large change [1] that there's a non-zero chance > of at least one glitch - let me know if you see any breakages. The mips*-*-* targets are not building. It looks like the mips reorg pass (pass_mips_machine_reorg2) in config/mips/mips.c was not converted and/or was not converted correctly. Steve Ellcey sell...@mips.com
Passes are now C++ classes (was Re: [PATCH 03/11] Handwritten part of conversion of passes to C++ classes)
On Mon, 2013-07-29 at 15:41 -0600, Jeff Law wrote: > On 07/26/2013 09:04 AM, David Malcolm wrote: > > This patch is the hand-written part of the conversion of passes from > > C structs to C++ classes. It does not work without the subsequent > > autogenerated part, which is huge. > [ ... ] > With the changes from pipeline -> pass_manager this is fine. As is the > follow-up autogenerated patch. I've committed this and the other patches that needed to go with it to trunk, with the name fix, having successfully bootstrapped and tested them on x86_64-unknown-linux-gnu - so opt_pass is now a C++ class hierarchy, allowing for pass instances to hold pass-specific data (albeit not GTY refs yet), and the future possibility of multiple pass managers and pass pipelines (e.g. for embedding GCC for JIT-compilation). Specifically, I fixed the names and rebased against master and tested repeatedly (for the later GC patches, which are still being reviewed). I refreshed the "03/11" patch (which became r201505) to resolve a trivial conflict in cgraphunit.c, and then bootstrapped and tested the series of 5 patches against r201494 on x86_64-unknown-linux-gnu (RHEL 6.4 in fact). It successfully built all 3 stages, and gave the same testing results as an unpatched build of r201494. Given that, I've committed the following to trunk: * r201505: this patch (aka 03/11; handwritten part of conversion, approved above by Jeff) * r201506: the zero-initialization of pass_manager (aka 3.1/11; approved by rth in http://gcc.gnu.org/ml/gcc-patches/2013-08/msg00147.html ) * r201508: the big autogenerated patch (aka 04/11; approved above by Jeff) * r201509: adding -fno-rtti when building plugins in the testsuite (aka 05/11; approved by Jeff in http://gcc.gnu.org/ml/gcc-patches/2013-07/msg01433.html ) * r201511: aka 06/11; using pass->clone and fixing pass switch numbering (approved by rth in http://gcc.gnu.org/ml/gcc-patches/2013-08/msg00054.html ) Note that r201505 and r201506 don't compile by themselves; they need r201508 to update all of the passes to the new internal API, and r201509 and r201511 are then needed otherwise you'll see regressions in the testsuite. Given all of the above testing I'm reasonably confident that this works. However this is such a large change [1] that there's a non-zero chance of at least one glitch - let me know if you see any breakages. One glitch that I ran into when applying the patches to svn was that I saw some end-of-file whitespace changes in some of the gcc/testsuite files: I'm not sure how those crept in, but I used "svn revert" by hand on them before committing to ensure that only the files I was expecting to change were changed. Some of followup patches in the series are now approved, some aren't, so I'm next going to bootstrap/test/commit the approved ones as appropriate. Dave [1] 127 files changed, 10227 insertions(+), 4465 deletions(-)
