| Message ID | 26d623c5117d22fbaea2ce1a445ba2d32df437ad.1619537141.git.wschmidt@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | Replace the Power target-specific built-in machinery | expand |
Hi! I'd like to ping this specific patch from the series, which is the only one remaining that affects common code. I confess that I don't know whom to ask for a review for gengtype; I didn't get any good ideas from MAINTAINERS. If you know of a good reviewer candidate, please CC them. In any case, this is a reasonably straightforward patch. It allows adding generated header files to be identified as "./header.h" and included in the files to be scanned by gengtype for GC roots. Thank you! Bill On 4/27/21 10:32 AM, Bill Schmidt wrote: > Currently gengtype supports scanning target-specific files for GC roots, > but those files must exist in the source tree. This patch extends the > support to include header files generated into the build directory. It > also allows targets to specify build dependencies for s-gtype to ensure > the built headers are up to date prior to running gengtype. > > 2021-04-02 Bill Schmidt <wschmidt@linux.ibm.com> > > gcc/ > * Makefile.in (EXTRA_GTYPE_DEPS): New variable. > (s-gtype): Depend on EXTRA_GTYPE_DEPS. > * gengtype-state.c (state_writer::write_state_files_list): Add a > parameter to the fileslist expression for the number of build > headers to scan. > (read_state_file_list): Detect build headers and strip the initial > "./" from their names. > * gengtype.c (build_headers): New global variable. > (num_build_headers): Likewise. > (open_base_files): Emit #include for each build header. > (main): Detect and count build headers. > * gengtype.h (build_headers): New extern variable. > (num_build_headers): Likewise. > --- > gcc/Makefile.in | 5 +++-- > gcc/gengtype-state.c | 29 +++++++++++++++++++++++------ > gcc/gengtype.c | 19 ++++++++++++++++--- > gcc/gengtype.h | 5 +++++ > 4 files changed, 47 insertions(+), 11 deletions(-) > > diff --git a/gcc/Makefile.in b/gcc/Makefile.in > index 2fd94fc7dba..1a253256042 100644 > --- a/gcc/Makefile.in > +++ b/gcc/Makefile.in > @@ -561,6 +561,7 @@ out_object_file=@out_object_file@ > OUT_FILE_DEPS= > common_out_file=$(srcdir)/common/config/@common_out_file@ > common_out_object_file=@common_out_object_file@ > +EXTRA_GTYPE_DEPS= > md_file=$(srcdir)/common.md $(srcdir)/config/@md_file@ > tm_file_list=@tm_file_list@ > tm_include_list=@tm_include_list@ > @@ -2740,8 +2741,8 @@ s-gtyp-input: Makefile > $(SHELL) $(srcdir)/../move-if-change tmp-gi.list gtyp-input.list > $(STAMP) s-gtyp-input > > -s-gtype: build/gengtype$(build_exeext) $(filter-out [%], $(GTFILES)) \ > - gtyp-input.list > +s-gtype: $(EXTRA_GTYPE_DEPS) build/gengtype$(build_exeext) \ > + $(filter-out [%], $(GTFILES)) gtyp-input.list > # First, parse all files and save a state file. > $(RUN_GEN) build/gengtype$(build_exeext) $(GENGTYPE_FLAGS) \ > -S $(srcdir) -I gtyp-input.list -w tmp-gtype.state > diff --git a/gcc/gengtype-state.c b/gcc/gengtype-state.c > index 891f2e18a61..be3549dce33 100644 > --- a/gcc/gengtype-state.c > +++ b/gcc/gengtype-state.c > @@ -1269,7 +1269,7 @@ state_writer::write_state_files_list (void) > int i = 0; > /* Write the list of files with their lang_bitmap. */ > begin_s_expr ("fileslist"); > - fprintf (state_file, "%d", (int) num_gt_files); > + fprintf (state_file, "%d %d", (int) num_gt_files, (int) num_build_headers); > for (i = 0; i < (int) num_gt_files; i++) > { > const char *cursrcrelpath = NULL; > @@ -2456,16 +2456,20 @@ read_state_files_list (void) > struct state_token_st *t0 = peek_state_token (0); > struct state_token_st *t1 = peek_state_token (1); > struct state_token_st *t2 = peek_state_token (2); > + struct state_token_st *t3 = peek_state_token (3); > > if (state_token_kind (t0) == STOK_LEFTPAR > && state_token_is_name (t1, "!fileslist") > - && state_token_kind (t2) == STOK_INTEGER) > + && state_token_kind (t2) == STOK_INTEGER > + && state_token_kind (t3) == STOK_INTEGER) > { > - int i = 0; > + int i = 0, j = 0; > num_gt_files = t2->stok_un.stok_num; > - next_state_tokens (3); > - t0 = t1 = t2 = NULL; > + num_build_headers = t3->stok_un.stok_num; > + next_state_tokens (4); > + t0 = t1 = t2 = t3 = NULL; > gt_files = XCNEWVEC (const input_file *, num_gt_files); > + build_headers = XCNEWVEC (const char *, num_build_headers); > for (i = 0; i < (int) num_gt_files; i++) > { > bool issrcfile = FALSE; > @@ -2498,7 +2502,20 @@ read_state_files_list (void) > free (fullpath); > } > else > - curgt = input_file_by_name (fnam); > + { > + curgt = input_file_by_name (fnam); > + /* Look for a header file created during the build, > + which looks like "./<filename>.h". */ > + int len = strlen (fnam); > + if (len >= 5 && fnam[0] == '.' && fnam[1] == '/' > + && fnam[len-2] == '.' && fnam[len-1] == 'h') > + { > + char *buf = (char *) xmalloc (len - 1); > + /* Strip the leading "./" from the filename. */ > + strcpy (buf, &fnam[2]); > + build_headers[j++] = buf; > + } > + } > set_lang_bitmap (curgt, bmap); > gt_files[i] = curgt; > next_state_tokens (2); > diff --git a/gcc/gengtype.c b/gcc/gengtype.c > index 98d4626f87e..57dc6e9fbe8 100644 > --- a/gcc/gengtype.c > +++ b/gcc/gengtype.c > @@ -143,6 +143,11 @@ get_ultimate_base_class (type_p s) > const input_file **gt_files; > size_t num_gt_files; > > +/* Table of headers to be included in gtype-desc.c that are generated > + during the build. These are identified as "./<filename>.h". */ > +const char** build_headers; > +size_t num_build_headers; > + > /* A number of places use the name of this "gengtype.c" file for a > location for things that we can't rely on the source to define. > Make sure we can still use pointer comparison on filenames. */ > @@ -1736,6 +1741,8 @@ open_base_files (void) > gtype_desc_c = create_file ("GCC", "gtype-desc.c"); > for (ifp = ifiles; *ifp; ifp++) > oprintf (gtype_desc_c, "#include \"%s\"\n", *ifp); > + for (int j = 0; j < (int) num_build_headers; j++) > + oprintf (gtype_desc_c, "#include \"%s\"\n", build_headers[j]); > > /* Make sure we handle "cfun" specially. */ > oprintf (gtype_desc_c, "\n/* See definition in function.h. */\n"); > @@ -5215,11 +5222,17 @@ main (int argc, char **argv) > &pos)); > #undef POS_HERE > read_input_list (inputlist); > + num_build_headers = 0; > for (i = 0; i < num_gt_files; i++) > { > - parse_file (get_input_file_name (gt_files[i])); > - DBGPRINTF ("parsed file #%d %s", > - (int) i, get_input_file_name (gt_files[i])); > + const char *fname = get_input_file_name (gt_files[i]); > + parse_file (fname); > + DBGPRINTF ("parsed file #%d %s", (int) i, fname); > + /* Check if this is a header file generated during the build. */ > + int len = strlen (fname); > + if (len >= 5 && fname[0] == '.' && fname[1] == '/' > + && fname[len-2] == '.' && fname[len-1] == 'h') > + num_build_headers++; > } > if (verbosity_level >= 1) > printf ("%s parsed %d files with %d GTY types\n", > diff --git a/gcc/gengtype.h b/gcc/gengtype.h > index 4fe8f0f7232..ca348a3b4b1 100644 > --- a/gcc/gengtype.h > +++ b/gcc/gengtype.h > @@ -55,6 +55,11 @@ struct fileloc > extern const input_file** gt_files; > extern size_t num_gt_files; > > +/* Table of headers to be included in gtype-desc.c that are generated > + during the build. These are identified as "./<filename>.h". */ > +extern const char** build_headers; > +extern size_t num_build_headers; > + > /* A number of places use the name of this "gengtype.c" file for a > location for things that we can't rely on the source to define. We > also need to refer to the "system.h" file specifically. These two
Hi! On Tue, Apr 27, 2021 at 10:32:37AM -0500, Bill Schmidt via Gcc-patches wrote: > --- a/gcc/gengtype-state.c > +++ b/gcc/gengtype-state.c > @@ -1269,7 +1269,7 @@ state_writer::write_state_files_list (void) > int i = 0; > /* Write the list of files with their lang_bitmap. */ > begin_s_expr ("fileslist"); > - fprintf (state_file, "%d", (int) num_gt_files); > + fprintf (state_file, "%d %d", (int) num_gt_files, (int) num_build_headers); Please use %zd instead, and don't cast? We require a moderately new host compiler nowadays :-) > for (i = 0; i < (int) num_gt_files; i++) For this one you can make i itself a size_t. Remember: all explicit casts are evil (and just some are useful). Alternatively you can make num_gt_files (and your new num_build_headers) itself an int: it's not like we could have 2G of those anyway. > --- a/gcc/gengtype.h > +++ b/gcc/gengtype.h > @@ -55,6 +55,11 @@ struct fileloc > extern const input_file** gt_files; > extern size_t num_gt_files; > > +/* Table of headers to be included in gtype-desc.c that are generated > + during the build. These are identified as "./<filename>.h". */ > +extern const char** build_headers; extern const char **build_headers; (I hope someone who can approve this will review it.) Segher
On Tue, May 11, 2021 at 11:01:22AM -0500, Bill Schmidt wrote: > Hi! I'd like to ping this specific patch from the series, which is the > only one remaining that affects common code. I confess that I don't > know whom to ask for a review for gengtype; I didn't get any good ideas > from MAINTAINERS. If you know of a good reviewer candidate, please CC > them. Richard is listed as the "gen* on machine desc" maintainer, that might be the closest to this. cc:ed. Segher
On 5/20/21 5:24 PM, Segher Boessenkool wrote: > On Tue, May 11, 2021 at 11:01:22AM -0500, Bill Schmidt wrote: >> Hi! I'd like to ping this specific patch from the series, which is the >> only one remaining that affects common code. I confess that I don't >> know whom to ask for a review for gengtype; I didn't get any good ideas >> from MAINTAINERS. If you know of a good reviewer candidate, please CC >> them. > Richard is listed as the "gen* on machine desc" maintainer, that might > be the closest to this. cc:ed. Hi, Richard -- any thoughts on this patch? https://gcc.gnu.org/pipermail/gcc-patches/2021-April/568841.html Thanks! Bill > > > Segher
Bill Schmidt via Gcc-patches <gcc-patches@gcc.gnu.org> writes: > On 5/20/21 5:24 PM, Segher Boessenkool wrote: >> On Tue, May 11, 2021 at 11:01:22AM -0500, Bill Schmidt wrote: >>> Hi! I'd like to ping this specific patch from the series, which is the >>> only one remaining that affects common code. I confess that I don't >>> know whom to ask for a review for gengtype; I didn't get any good ideas >>> from MAINTAINERS. If you know of a good reviewer candidate, please CC >>> them. >> Richard is listed as the "gen* on machine desc" maintainer, that might >> be the closest to this. cc:ed. > > Hi, Richard -- any thoughts on this patch? > > https://gcc.gnu.org/pipermail/gcc-patches/2021-April/568841.html I don't really know gengtype.c, sorry. (The gen* thing was for the md generators, not genmatch and gengtype.) Richard
On 6/7/21 5:39 AM, Richard Sandiford wrote: > Bill Schmidt via Gcc-patches <gcc-patches@gcc.gnu.org> writes: >> On 5/20/21 5:24 PM, Segher Boessenkool wrote: >>> On Tue, May 11, 2021 at 11:01:22AM -0500, Bill Schmidt wrote: >>>> Hi! I'd like to ping this specific patch from the series, which is the >>>> only one remaining that affects common code. I confess that I don't >>>> know whom to ask for a review for gengtype; I didn't get any good ideas >>>> from MAINTAINERS. If you know of a good reviewer candidate, please CC >>>> them. >>> Richard is listed as the "gen* on machine desc" maintainer, that might >>> be the closest to this. cc:ed. >> Hi, Richard -- any thoughts on this patch? >> >> https://gcc.gnu.org/pipermail/gcc-patches/2021-April/568841.html > I don't really know gengtype.c, sorry. (The gen* thing was for > the md generators, not genmatch and gengtype.) > > Richard OK, thanks, Richard! Richi, from git blame, it appears everyone that used to know about gengtype has moved on. Would you be able to give a quick peek at this? It's a pretty uninteresting patch, all in all. :-) Thanks, Bill
On Mon, Jun 7, 2021 at 2:35 PM Bill Schmidt <wschmidt@linux.ibm.com> wrote: > > On 6/7/21 5:39 AM, Richard Sandiford wrote: > > Bill Schmidt via Gcc-patches <gcc-patches@gcc.gnu.org> writes: > >> On 5/20/21 5:24 PM, Segher Boessenkool wrote: > >>> On Tue, May 11, 2021 at 11:01:22AM -0500, Bill Schmidt wrote: > >>>> Hi! I'd like to ping this specific patch from the series, which is the > >>>> only one remaining that affects common code. I confess that I don't > >>>> know whom to ask for a review for gengtype; I didn't get any good ideas > >>>> from MAINTAINERS. If you know of a good reviewer candidate, please CC > >>>> them. > >>> Richard is listed as the "gen* on machine desc" maintainer, that might > >>> be the closest to this. cc:ed. > >> Hi, Richard -- any thoughts on this patch? > >> > >> https://gcc.gnu.org/pipermail/gcc-patches/2021-April/568841.html > > I don't really know gengtype.c, sorry. (The gen* thing was for > > the md generators, not genmatch and gengtype.) > > > > Richard > > OK, thanks, Richard! > > Richi, from git blame, it appears everyone that used to know about > gengtype has moved on. Ouch. > Would you be able to give a quick peek at this? > It's a pretty uninteresting patch, all in all. :-) Some maybe obvious issue - what about DOS-style path hosts? You seem to build ../ strings to point to parent dirs... I'm not sure what we do elsewhere - I suppose we arrange for appropriate -I command line arguments? Richard. > > Thanks, > Bill >
On 6/7/21 8:36 AM, Richard Biener wrote: > > Some maybe obvious issue - what about DOS-style path hosts? > You seem to build ../ strings to point to parent dirs... I'm not sure > what we do elsewhere - I suppose we arrange for appropriate > -I command line arguments? > Well, actually it's just using "./" to identify the build directory, though I see what you mean about potential Linux bias. There is precedent for this syntax identifying the build directory in config.gcc for target macro files: # tm_file A list of target macro files, if different from # "$cpu_type/$cpu_type.h". Usually it's constructed # per target in a way like this: # tm_file="${tm_file} dbxelf.h elfos.h ${cpu_type.h}/elf.h" # Note that the preferred order is: # - specific target header "${cpu_type}/${cpu_type.h}" # - generic headers like dbxelf.h elfos.h, etc. # - specializing target headers like ${cpu_type.h}/elf.h # This helps to keep OS specific stuff out of the CPU # defining header ${cpu_type}/${cpu_type.h}. # # It is possible to include automatically-generated # build-directory files by prefixing them with "./". # All other files should relative to $srcdir/config. ...so I thought I would try to be consistent with this change. In patch 0025 I use this as follows: --- a/gcc/config.gcc +++ b/gcc/config.gcc @@ -491,6 +491,7 @@ powerpc*-*-*) extra_options="${extra_options} g.opt fused-madd.opt rs6000/rs6000-tables.opt" target_gtfiles="$target_gtfiles \$(srcdir)/config/rs6000/rs6000-logue.c \$(srcdir)/config/rs6000/rs6000-call.c" target_gtfiles="$target_gtfiles \$(srcdir)/config/rs6000/rs6000-pcrel-opt.c" + target_gtfiles="$target_gtfiles ./rs6000-builtins.h" ;; pru-*-*) cpu_type=pru I'm open to trying to do something different if you think that's appropriate. Thanks for your help with this! Bill
On Mon, Jun 7, 2021 at 5:38 PM Bill Schmidt <wschmidt@linux.ibm.com> wrote: > > On 6/7/21 8:36 AM, Richard Biener wrote: > > > > Some maybe obvious issue - what about DOS-style path hosts? > > You seem to build ../ strings to point to parent dirs... I'm not sure > > what we do elsewhere - I suppose we arrange for appropriate > > -I command line arguments? > > > Well, actually it's just using "./" to identify the build directory, > though I see what you mean about potential Linux bias. There is > precedent for this syntax identifying the build directory in config.gcc > for target macro files: > > # tm_file A list of target macro files, if different from > # "$cpu_type/$cpu_type.h". Usually it's constructed > # per target in a way like this: > # tm_file="${tm_file} dbxelf.h elfos.h > ${cpu_type.h}/elf.h" > # Note that the preferred order is: > # - specific target header > "${cpu_type}/${cpu_type.h}" > # - generic headers like dbxelf.h elfos.h, etc. > # - specializing target headers like > ${cpu_type.h}/elf.h > # This helps to keep OS specific stuff out of the CPU > # defining header ${cpu_type}/${cpu_type.h}. > # > # It is possible to include automatically-generated > # build-directory files by prefixing them with "./". > # All other files should relative to $srcdir/config. > > ...so I thought I would try to be consistent with this change. In patch > 0025 I use this as follows: > > --- a/gcc/config.gcc > +++ b/gcc/config.gcc > @@ -491,6 +491,7 @@ powerpc*-*-*) > extra_options="${extra_options} g.opt fused-madd.opt > rs6000/rs6000-tables.opt" > target_gtfiles="$target_gtfiles > \$(srcdir)/config/rs6000/rs6000-logue.c > \$(srcdir)/config/rs6000/rs6000-call.c" > target_gtfiles="$target_gtfiles > \$(srcdir)/config/rs6000/rs6000-pcrel-opt.c" > + target_gtfiles="$target_gtfiles ./rs6000-builtins.h" > ;; > pru-*-*) > cpu_type=pru > > I'm open to trying to do something different if you think that's > appropriate. Well, I'm not sure whether/how to resolve this. You could try building a cross to powerpc-linux from a x86_64-mingw host ... maybe there's one on the CF? Or some of your fellow RedHat people have access to mingw or the like envs to try whether it just works with your change ... Otherwise it looks OK. Richard. > Thanks for your help with this! > > Bill >
On 6/7/21 12:45 PM, Richard Biener wrote: > On Mon, Jun 7, 2021 at 5:38 PM Bill Schmidt <wschmidt@linux.ibm.com> wrote: >> On 6/7/21 8:36 AM, Richard Biener wrote: >>> Some maybe obvious issue - what about DOS-style path hosts? >>> You seem to build ../ strings to point to parent dirs... I'm not sure >>> what we do elsewhere - I suppose we arrange for appropriate >>> -I command line arguments? >>> >> Well, actually it's just using "./" to identify the build directory, >> though I see what you mean about potential Linux bias. There is >> precedent for this syntax identifying the build directory in config.gcc >> for target macro files: >> >> # tm_file A list of target macro files, if different from >> # "$cpu_type/$cpu_type.h". Usually it's constructed >> # per target in a way like this: >> # tm_file="${tm_file} dbxelf.h elfos.h >> ${cpu_type.h}/elf.h" >> # Note that the preferred order is: >> # - specific target header >> "${cpu_type}/${cpu_type.h}" >> # - generic headers like dbxelf.h elfos.h, etc. >> # - specializing target headers like >> ${cpu_type.h}/elf.h >> # This helps to keep OS specific stuff out of the CPU >> # defining header ${cpu_type}/${cpu_type.h}. >> # >> # It is possible to include automatically-generated >> # build-directory files by prefixing them with "./". >> # All other files should relative to $srcdir/config. >> >> ...so I thought I would try to be consistent with this change. In patch >> 0025 I use this as follows: >> >> --- a/gcc/config.gcc >> +++ b/gcc/config.gcc >> @@ -491,6 +491,7 @@ powerpc*-*-*) >> extra_options="${extra_options} g.opt fused-madd.opt >> rs6000/rs6000-tables.opt" >> target_gtfiles="$target_gtfiles >> \$(srcdir)/config/rs6000/rs6000-logue.c >> \$(srcdir)/config/rs6000/rs6000-call.c" >> target_gtfiles="$target_gtfiles >> \$(srcdir)/config/rs6000/rs6000-pcrel-opt.c" >> + target_gtfiles="$target_gtfiles ./rs6000-builtins.h" >> ;; >> pru-*-*) >> cpu_type=pru >> >> I'm open to trying to do something different if you think that's >> appropriate. > Well, I'm not sure whether/how to resolve this. You could try > building a cross to powerpc-linux from a x86_64-mingw host ... > maybe there's one on the CF? Or some of your fellow RedHat > people have access to mingw or the like envs to try whether it > just works with your change ... > > Otherwise it looks OK. I'll see what I can find. Thanks again for reviewing the patch! Bill > > Richard. > >> Thanks for your help with this! >> >> Bill >>
On 6/7/21 12:48 PM, Bill Schmidt wrote: > On 6/7/21 12:45 PM, Richard Biener wrote: >> On Mon, Jun 7, 2021 at 5:38 PM Bill Schmidt <wschmidt@linux.ibm.com> >> wrote: >>> On 6/7/21 8:36 AM, Richard Biener wrote: >>>> Some maybe obvious issue - what about DOS-style path hosts? >>>> You seem to build ../ strings to point to parent dirs... I'm not sure >>>> what we do elsewhere - I suppose we arrange for appropriate >>>> -I command line arguments? >>>> >>> Well, actually it's just using "./" to identify the build directory, >>> though I see what you mean about potential Linux bias. There is >>> precedent for this syntax identifying the build directory in config.gcc >>> for target macro files: >>> >>> # tm_file A list of target macro files, if different from >>> # "$cpu_type/$cpu_type.h". Usually it's >>> constructed >>> # per target in a way like this: >>> # tm_file="${tm_file} dbxelf.h elfos.h >>> ${cpu_type.h}/elf.h" >>> # Note that the preferred order is: >>> # - specific target header >>> "${cpu_type}/${cpu_type.h}" >>> # - generic headers like dbxelf.h elfos.h, etc. >>> # - specializing target headers like >>> ${cpu_type.h}/elf.h >>> # This helps to keep OS specific stuff out of >>> the CPU >>> # defining header ${cpu_type}/${cpu_type.h}. >>> # >>> # It is possible to include >>> automatically-generated >>> # build-directory files by prefixing them with >>> "./". >>> # All other files should relative to >>> $srcdir/config. >>> >>> ...so I thought I would try to be consistent with this change. In patch >>> 0025 I use this as follows: >>> >>> --- a/gcc/config.gcc >>> +++ b/gcc/config.gcc >>> @@ -491,6 +491,7 @@ powerpc*-*-*) >>> extra_options="${extra_options} g.opt fused-madd.opt >>> rs6000/rs6000-tables.opt" >>> target_gtfiles="$target_gtfiles >>> \$(srcdir)/config/rs6000/rs6000-logue.c >>> \$(srcdir)/config/rs6000/rs6000-call.c" >>> target_gtfiles="$target_gtfiles >>> \$(srcdir)/config/rs6000/rs6000-pcrel-opt.c" >>> + target_gtfiles="$target_gtfiles ./rs6000-builtins.h" >>> ;; >>> pru-*-*) >>> cpu_type=pru >>> >>> I'm open to trying to do something different if you think that's >>> appropriate. >> Well, I'm not sure whether/how to resolve this. You could try >> building a cross to powerpc-linux from a x86_64-mingw host ... >> maybe there's one on the CF? Or some of your fellow RedHat >> people have access to mingw or the like envs to try whether it >> just works with your change ... >> >> Otherwise it looks OK. > > I'll see what I can find. Thanks again for reviewing the patch! Hm. Ultimately, I think the cross compiler case is doomed unless mingw already handles converting forward slashes to back slashes. There's no single syntax that works on both Windows and Linux. (There's no mingw server in the compile farm to play with.) I'm inclined to accept both "./" and ".\" for native builds, and kick the can down the road beyond that. What do you think? Bill > > Bill > > >> >> Richard. >> >>> Thanks for your help with this! >>> >>> Bill >>>
On Tue, Jun 8, 2021 at 10:45 PM Bill Schmidt <wschmidt@linux.ibm.com> wrote: > > On 6/7/21 12:48 PM, Bill Schmidt wrote: > > On 6/7/21 12:45 PM, Richard Biener wrote: > >> On Mon, Jun 7, 2021 at 5:38 PM Bill Schmidt <wschmidt@linux.ibm.com> > >> wrote: > >>> On 6/7/21 8:36 AM, Richard Biener wrote: > >>>> Some maybe obvious issue - what about DOS-style path hosts? > >>>> You seem to build ../ strings to point to parent dirs... I'm not sure > >>>> what we do elsewhere - I suppose we arrange for appropriate > >>>> -I command line arguments? > >>>> > >>> Well, actually it's just using "./" to identify the build directory, > >>> though I see what you mean about potential Linux bias. There is > >>> precedent for this syntax identifying the build directory in config.gcc > >>> for target macro files: > >>> > >>> # tm_file A list of target macro files, if different from > >>> # "$cpu_type/$cpu_type.h". Usually it's > >>> constructed > >>> # per target in a way like this: > >>> # tm_file="${tm_file} dbxelf.h elfos.h > >>> ${cpu_type.h}/elf.h" > >>> # Note that the preferred order is: > >>> # - specific target header > >>> "${cpu_type}/${cpu_type.h}" > >>> # - generic headers like dbxelf.h elfos.h, etc. > >>> # - specializing target headers like > >>> ${cpu_type.h}/elf.h > >>> # This helps to keep OS specific stuff out of > >>> the CPU > >>> # defining header ${cpu_type}/${cpu_type.h}. > >>> # > >>> # It is possible to include > >>> automatically-generated > >>> # build-directory files by prefixing them with > >>> "./". > >>> # All other files should relative to > >>> $srcdir/config. > >>> > >>> ...so I thought I would try to be consistent with this change. In patch > >>> 0025 I use this as follows: > >>> > >>> --- a/gcc/config.gcc > >>> +++ b/gcc/config.gcc > >>> @@ -491,6 +491,7 @@ powerpc*-*-*) > >>> extra_options="${extra_options} g.opt fused-madd.opt > >>> rs6000/rs6000-tables.opt" > >>> target_gtfiles="$target_gtfiles > >>> \$(srcdir)/config/rs6000/rs6000-logue.c > >>> \$(srcdir)/config/rs6000/rs6000-call.c" > >>> target_gtfiles="$target_gtfiles > >>> \$(srcdir)/config/rs6000/rs6000-pcrel-opt.c" > >>> + target_gtfiles="$target_gtfiles ./rs6000-builtins.h" > >>> ;; > >>> pru-*-*) > >>> cpu_type=pru > >>> > >>> I'm open to trying to do something different if you think that's > >>> appropriate. > >> Well, I'm not sure whether/how to resolve this. You could try > >> building a cross to powerpc-linux from a x86_64-mingw host ... > >> maybe there's one on the CF? Or some of your fellow RedHat > >> people have access to mingw or the like envs to try whether it > >> just works with your change ... > >> > >> Otherwise it looks OK. > > > > I'll see what I can find. Thanks again for reviewing the patch! > > > Hm. Ultimately, I think the cross compiler case is doomed unless mingw > already handles converting forward slashes to back slashes. There's no > single syntax that works on both Windows and Linux. (There's no mingw > server in the compile farm to play with.) > > I'm inclined to accept both "./" and ".\" for native builds, and kick > the can down the road beyond that. What do you think? Can't you use PATH_SEPARATOR somehow? See file-find.c / incpath.c or gcc.c for uses and system.h for where it is defined. Richard. > > Bill > > > > > Bill > > > > > >> > >> Richard. > >> > >>> Thanks for your help with this! > >>> > >>> Bill > >>>
On Wed, Jun 9, 2021 at 12:53 PM Richard Biener <richard.guenther@gmail.com> wrote: > > On Tue, Jun 8, 2021 at 10:45 PM Bill Schmidt <wschmidt@linux.ibm.com> wrote: > > > > On 6/7/21 12:48 PM, Bill Schmidt wrote: > > > On 6/7/21 12:45 PM, Richard Biener wrote: > > >> On Mon, Jun 7, 2021 at 5:38 PM Bill Schmidt <wschmidt@linux.ibm.com> > > >> wrote: > > >>> On 6/7/21 8:36 AM, Richard Biener wrote: > > >>>> Some maybe obvious issue - what about DOS-style path hosts? > > >>>> You seem to build ../ strings to point to parent dirs... I'm not sure > > >>>> what we do elsewhere - I suppose we arrange for appropriate > > >>>> -I command line arguments? > > >>>> > > >>> Well, actually it's just using "./" to identify the build directory, > > >>> though I see what you mean about potential Linux bias. There is > > >>> precedent for this syntax identifying the build directory in config.gcc > > >>> for target macro files: > > >>> > > >>> # tm_file A list of target macro files, if different from > > >>> # "$cpu_type/$cpu_type.h". Usually it's > > >>> constructed > > >>> # per target in a way like this: > > >>> # tm_file="${tm_file} dbxelf.h elfos.h > > >>> ${cpu_type.h}/elf.h" > > >>> # Note that the preferred order is: > > >>> # - specific target header > > >>> "${cpu_type}/${cpu_type.h}" > > >>> # - generic headers like dbxelf.h elfos.h, etc. > > >>> # - specializing target headers like > > >>> ${cpu_type.h}/elf.h > > >>> # This helps to keep OS specific stuff out of > > >>> the CPU > > >>> # defining header ${cpu_type}/${cpu_type.h}. > > >>> # > > >>> # It is possible to include > > >>> automatically-generated > > >>> # build-directory files by prefixing them with > > >>> "./". > > >>> # All other files should relative to > > >>> $srcdir/config. > > >>> > > >>> ...so I thought I would try to be consistent with this change. In patch > > >>> 0025 I use this as follows: > > >>> > > >>> --- a/gcc/config.gcc > > >>> +++ b/gcc/config.gcc > > >>> @@ -491,6 +491,7 @@ powerpc*-*-*) > > >>> extra_options="${extra_options} g.opt fused-madd.opt > > >>> rs6000/rs6000-tables.opt" > > >>> target_gtfiles="$target_gtfiles > > >>> \$(srcdir)/config/rs6000/rs6000-logue.c > > >>> \$(srcdir)/config/rs6000/rs6000-call.c" > > >>> target_gtfiles="$target_gtfiles > > >>> \$(srcdir)/config/rs6000/rs6000-pcrel-opt.c" > > >>> + target_gtfiles="$target_gtfiles ./rs6000-builtins.h" > > >>> ;; > > >>> pru-*-*) > > >>> cpu_type=pru > > >>> > > >>> I'm open to trying to do something different if you think that's > > >>> appropriate. > > >> Well, I'm not sure whether/how to resolve this. You could try > > >> building a cross to powerpc-linux from a x86_64-mingw host ... > > >> maybe there's one on the CF? Or some of your fellow RedHat > > >> people have access to mingw or the like envs to try whether it > > >> just works with your change ... > > >> > > >> Otherwise it looks OK. > > > > > > I'll see what I can find. Thanks again for reviewing the patch! > > > > > > Hm. Ultimately, I think the cross compiler case is doomed unless mingw > > already handles converting forward slashes to back slashes. There's no > > single syntax that works on both Windows and Linux. (There's no mingw > > server in the compile farm to play with.) > > > > I'm inclined to accept both "./" and ".\" for native builds, and kick > > the can down the road beyond that. What do you think? > > Can't you use PATH_SEPARATOR somehow? See file-find.c / incpath.c > or gcc.c for uses and system.h for where it is defined. Err - DIR_SEPARATOR of course. Richard. > Richard. > > > > > Bill > > > > > > > > Bill > > > > > > > > >> > > >> Richard. > > >> > > >>> Thanks for your help with this! > > >>> > > >>> Bill > > >>>
On 6/9/21 5:54 AM, Richard Biener wrote: > On Wed, Jun 9, 2021 at 12:53 PM Richard Biener > <richard.guenther@gmail.com> wrote: >> On Tue, Jun 8, 2021 at 10:45 PM Bill Schmidt <wschmidt@linux.ibm.com> wrote: >>> On 6/7/21 12:48 PM, Bill Schmidt wrote: >>>> On 6/7/21 12:45 PM, Richard Biener wrote: >>>>> On Mon, Jun 7, 2021 at 5:38 PM Bill Schmidt <wschmidt@linux.ibm.com> >>>>> wrote: >>>>>> On 6/7/21 8:36 AM, Richard Biener wrote: >>>>>>> Some maybe obvious issue - what about DOS-style path hosts? >>>>>>> You seem to build ../ strings to point to parent dirs... I'm not sure >>>>>>> what we do elsewhere - I suppose we arrange for appropriate >>>>>>> -I command line arguments? >>>>>>> >>>>>> Well, actually it's just using "./" to identify the build directory, >>>>>> though I see what you mean about potential Linux bias. There is >>>>>> precedent for this syntax identifying the build directory in config.gcc >>>>>> for target macro files: >>>>>> >>>>>> # tm_file A list of target macro files, if different from >>>>>> # "$cpu_type/$cpu_type.h". Usually it's >>>>>> constructed >>>>>> # per target in a way like this: >>>>>> # tm_file="${tm_file} dbxelf.h elfos.h >>>>>> ${cpu_type.h}/elf.h" >>>>>> # Note that the preferred order is: >>>>>> # - specific target header >>>>>> "${cpu_type}/${cpu_type.h}" >>>>>> # - generic headers like dbxelf.h elfos.h, etc. >>>>>> # - specializing target headers like >>>>>> ${cpu_type.h}/elf.h >>>>>> # This helps to keep OS specific stuff out of >>>>>> the CPU >>>>>> # defining header ${cpu_type}/${cpu_type.h}. >>>>>> # >>>>>> # It is possible to include >>>>>> automatically-generated >>>>>> # build-directory files by prefixing them with >>>>>> "./". >>>>>> # All other files should relative to >>>>>> $srcdir/config. >>>>>> >>>>>> ...so I thought I would try to be consistent with this change. In patch >>>>>> 0025 I use this as follows: >>>>>> >>>>>> --- a/gcc/config.gcc >>>>>> +++ b/gcc/config.gcc >>>>>> @@ -491,6 +491,7 @@ powerpc*-*-*) >>>>>> extra_options="${extra_options} g.opt fused-madd.opt >>>>>> rs6000/rs6000-tables.opt" >>>>>> target_gtfiles="$target_gtfiles >>>>>> \$(srcdir)/config/rs6000/rs6000-logue.c >>>>>> \$(srcdir)/config/rs6000/rs6000-call.c" >>>>>> target_gtfiles="$target_gtfiles >>>>>> \$(srcdir)/config/rs6000/rs6000-pcrel-opt.c" >>>>>> + target_gtfiles="$target_gtfiles ./rs6000-builtins.h" >>>>>> ;; >>>>>> pru-*-*) >>>>>> cpu_type=pru >>>>>> >>>>>> I'm open to trying to do something different if you think that's >>>>>> appropriate. >>>>> Well, I'm not sure whether/how to resolve this. You could try >>>>> building a cross to powerpc-linux from a x86_64-mingw host ... >>>>> maybe there's one on the CF? Or some of your fellow RedHat >>>>> people have access to mingw or the like envs to try whether it >>>>> just works with your change ... >>>>> >>>>> Otherwise it looks OK. >>>> I'll see what I can find. Thanks again for reviewing the patch! >>> >>> Hm. Ultimately, I think the cross compiler case is doomed unless mingw >>> already handles converting forward slashes to back slashes. There's no >>> single syntax that works on both Windows and Linux. (There's no mingw >>> server in the compile farm to play with.) >>> >>> I'm inclined to accept both "./" and ".\" for native builds, and kick >>> the can down the road beyond that. What do you think? >> Can't you use PATH_SEPARATOR somehow? See file-find.c / incpath.c >> or gcc.c for uses and system.h for where it is defined. > Err - DIR_SEPARATOR of course. Ah -- following the breadcrumbs a little further, it appears that IS_DIR_SEPARATOR is the proper way to handle both Linux- and Windows-style syntax. Thanks for the pointer! That should work. Will test. Bill > > Richard. > >> Richard. >> >>> Bill >>> >>>> Bill >>>> >>>> >>>>> Richard. >>>>> >>>>>> Thanks for your help with this! >>>>>> >>>>>> Bill >>>>>>
diff --git a/gcc/Makefile.in b/gcc/Makefile.in index 2fd94fc7dba..1a253256042 100644 --- a/gcc/Makefile.in +++ b/gcc/Makefile.in @@ -561,6 +561,7 @@ out_object_file=@out_object_file@ OUT_FILE_DEPS= common_out_file=$(srcdir)/common/config/@common_out_file@ common_out_object_file=@common_out_object_file@ +EXTRA_GTYPE_DEPS= md_file=$(srcdir)/common.md $(srcdir)/config/@md_file@ tm_file_list=@tm_file_list@ tm_include_list=@tm_include_list@ @@ -2740,8 +2741,8 @@ s-gtyp-input: Makefile $(SHELL) $(srcdir)/../move-if-change tmp-gi.list gtyp-input.list $(STAMP) s-gtyp-input -s-gtype: build/gengtype$(build_exeext) $(filter-out [%], $(GTFILES)) \ - gtyp-input.list +s-gtype: $(EXTRA_GTYPE_DEPS) build/gengtype$(build_exeext) \ + $(filter-out [%], $(GTFILES)) gtyp-input.list # First, parse all files and save a state file. $(RUN_GEN) build/gengtype$(build_exeext) $(GENGTYPE_FLAGS) \ -S $(srcdir) -I gtyp-input.list -w tmp-gtype.state diff --git a/gcc/gengtype-state.c b/gcc/gengtype-state.c index 891f2e18a61..be3549dce33 100644 --- a/gcc/gengtype-state.c +++ b/gcc/gengtype-state.c @@ -1269,7 +1269,7 @@ state_writer::write_state_files_list (void) int i = 0; /* Write the list of files with their lang_bitmap. */ begin_s_expr ("fileslist"); - fprintf (state_file, "%d", (int) num_gt_files); + fprintf (state_file, "%d %d", (int) num_gt_files, (int) num_build_headers); for (i = 0; i < (int) num_gt_files; i++) { const char *cursrcrelpath = NULL; @@ -2456,16 +2456,20 @@ read_state_files_list (void) struct state_token_st *t0 = peek_state_token (0); struct state_token_st *t1 = peek_state_token (1); struct state_token_st *t2 = peek_state_token (2); + struct state_token_st *t3 = peek_state_token (3); if (state_token_kind (t0) == STOK_LEFTPAR && state_token_is_name (t1, "!fileslist") - && state_token_kind (t2) == STOK_INTEGER) + && state_token_kind (t2) == STOK_INTEGER + && state_token_kind (t3) == STOK_INTEGER) { - int i = 0; + int i = 0, j = 0; num_gt_files = t2->stok_un.stok_num; - next_state_tokens (3); - t0 = t1 = t2 = NULL; + num_build_headers = t3->stok_un.stok_num; + next_state_tokens (4); + t0 = t1 = t2 = t3 = NULL; gt_files = XCNEWVEC (const input_file *, num_gt_files); + build_headers = XCNEWVEC (const char *, num_build_headers); for (i = 0; i < (int) num_gt_files; i++) { bool issrcfile = FALSE; @@ -2498,7 +2502,20 @@ read_state_files_list (void) free (fullpath); } else - curgt = input_file_by_name (fnam); + { + curgt = input_file_by_name (fnam); + /* Look for a header file created during the build, + which looks like "./<filename>.h". */ + int len = strlen (fnam); + if (len >= 5 && fnam[0] == '.' && fnam[1] == '/' + && fnam[len-2] == '.' && fnam[len-1] == 'h') + { + char *buf = (char *) xmalloc (len - 1); + /* Strip the leading "./" from the filename. */ + strcpy (buf, &fnam[2]); + build_headers[j++] = buf; + } + } set_lang_bitmap (curgt, bmap); gt_files[i] = curgt; next_state_tokens (2); diff --git a/gcc/gengtype.c b/gcc/gengtype.c index 98d4626f87e..57dc6e9fbe8 100644 --- a/gcc/gengtype.c +++ b/gcc/gengtype.c @@ -143,6 +143,11 @@ get_ultimate_base_class (type_p s) const input_file **gt_files; size_t num_gt_files; +/* Table of headers to be included in gtype-desc.c that are generated + during the build. These are identified as "./<filename>.h". */ +const char** build_headers; +size_t num_build_headers; + /* A number of places use the name of this "gengtype.c" file for a location for things that we can't rely on the source to define. Make sure we can still use pointer comparison on filenames. */ @@ -1736,6 +1741,8 @@ open_base_files (void) gtype_desc_c = create_file ("GCC", "gtype-desc.c"); for (ifp = ifiles; *ifp; ifp++) oprintf (gtype_desc_c, "#include \"%s\"\n", *ifp); + for (int j = 0; j < (int) num_build_headers; j++) + oprintf (gtype_desc_c, "#include \"%s\"\n", build_headers[j]); /* Make sure we handle "cfun" specially. */ oprintf (gtype_desc_c, "\n/* See definition in function.h. */\n"); @@ -5215,11 +5222,17 @@ main (int argc, char **argv) &pos)); #undef POS_HERE read_input_list (inputlist); + num_build_headers = 0; for (i = 0; i < num_gt_files; i++) { - parse_file (get_input_file_name (gt_files[i])); - DBGPRINTF ("parsed file #%d %s", - (int) i, get_input_file_name (gt_files[i])); + const char *fname = get_input_file_name (gt_files[i]); + parse_file (fname); + DBGPRINTF ("parsed file #%d %s", (int) i, fname); + /* Check if this is a header file generated during the build. */ + int len = strlen (fname); + if (len >= 5 && fname[0] == '.' && fname[1] == '/' + && fname[len-2] == '.' && fname[len-1] == 'h') + num_build_headers++; } if (verbosity_level >= 1) printf ("%s parsed %d files with %d GTY types\n", diff --git a/gcc/gengtype.h b/gcc/gengtype.h index 4fe8f0f7232..ca348a3b4b1 100644 --- a/gcc/gengtype.h +++ b/gcc/gengtype.h @@ -55,6 +55,11 @@ struct fileloc extern const input_file** gt_files; extern size_t num_gt_files; +/* Table of headers to be included in gtype-desc.c that are generated + during the build. These are identified as "./<filename>.h". */ +extern const char** build_headers; +extern size_t num_build_headers; + /* A number of places use the name of this "gengtype.c" file for a location for things that we can't rely on the source to define. We also need to refer to the "system.h" file specifically. These two