| Message ID | 4f27468885fd84aba5f0dc2369a284f3cffa2b34.1623941441.git.wschmidt@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | Replace the Power target-specific builtin machinery | expand |
On Thu, Jun 17, 2021 at 10:19:07AM -0500, Bill Schmidt wrote: > 2021-06-07 Bill Schmidt <wschmidt@linux.ibm.com> > > gcc/ > * config.gcc (extra_objs): Include rs6000-builtins.o and > rs6000-c.o. The rs6000-c.o part needs an explanation, and probably should be a separate (bugfix) patch (and it needs backports?) The changelog entry should read * config.gcc (powerpc*-*-*): Include [...] in extra_objs. or similar. > * config/rs6000/t-rs6000 (rs6000-gen-builtins.o): New target. > (rbtree.o): Likewise. > (rs6000-gen-builtins): Likewise. > (rs6000-builtins.c): Likewise. > (rs6000-builtins.h): Likewise. > (rs6000.o): Add dependency. > (EXTRA_HEADERS): Add rs6000-vecdefines.h. > (rs6000-vecdefines.h): New target. > (rs6000-builtins.o): Likewise. > (rs6000-call.o): Add rs6000-builtins.h as a dependency. > (rs6000-c.o): Likewise. > +rs6000-gen-builtins.o: $(srcdir)/config/rs6000/rs6000-gen-builtins.c > + $(COMPILE) $(CXXFLAGS) $< > + $(POSTCOMPILE) > + > +rbtree.o: $(srcdir)/config/rs6000/rbtree.c > + $(COMPILE) $< > + $(POSTCOMPILE) Why does one need CXXFLAGS and the other does not? > +# TODO: Whenever GNU make 4.3 is the minimum required, we should use > +# grouped targets on this: That may be quite a while still. GNU make is the foundation of everything, so we cannot require very new versions of it ever. In the meantime, you can make all these targets depend on an intermediate target (that you mark with .INTERMEDIATE), and have that intermediate target have the dependencies. This is from version 3.74.3 and we require 3.80 already, so this is fine. > +EXTRA_HEADERS += rs6000-vecdefines.h > +rs6000-vecdefines.h : rs6000-builtins.c No space before the colon please. Segher
Hi Segher, On 7/21/21 1:58 PM, Segher Boessenkool wrote: > On Thu, Jun 17, 2021 at 10:19:07AM -0500, Bill Schmidt wrote: >> 2021-06-07 Bill Schmidt <wschmidt@linux.ibm.com> >> >> gcc/ >> * config.gcc (extra_objs): Include rs6000-builtins.o and >> rs6000-c.o. > The rs6000-c.o part needs an explanation, and probably should be a > separate (bugfix) patch (and it needs backports?) OK. Yeah, I'm a little surprised looking at this diff now...I wonder if it got removed by accident by some previous patch. I'll investigate. > > The changelog entry should read > * config.gcc (powerpc*-*-*): Include [...] in extra_objs. > or similar. OK, thanks! Will fix. > >> * config/rs6000/t-rs6000 (rs6000-gen-builtins.o): New target. >> (rbtree.o): Likewise. >> (rs6000-gen-builtins): Likewise. >> (rs6000-builtins.c): Likewise. >> (rs6000-builtins.h): Likewise. >> (rs6000.o): Add dependency. >> (EXTRA_HEADERS): Add rs6000-vecdefines.h. >> (rs6000-vecdefines.h): New target. >> (rs6000-builtins.o): Likewise. >> (rs6000-call.o): Add rs6000-builtins.h as a dependency. >> (rs6000-c.o): Likewise. >> +rs6000-gen-builtins.o: $(srcdir)/config/rs6000/rs6000-gen-builtins.c >> + $(COMPILE) $(CXXFLAGS) $< >> + $(POSTCOMPILE) >> + >> +rbtree.o: $(srcdir)/config/rs6000/rbtree.c >> + $(COMPILE) $< >> + $(POSTCOMPILE) > Why does one need CXXFLAGS and the other does not? Well, I'm as puzzled as you are. I'll remove the $(CXXFLAGS) and see if anything goes wrong to remind me...but I think it's just unnecessary. > >> +# TODO: Whenever GNU make 4.3 is the minimum required, we should use >> +# grouped targets on this: > That may be quite a while still. GNU make is the foundation of > everything, so we cannot require very new versions of it ever. > > In the meantime, you can make all these targets depend on an > intermediate target (that you mark with .INTERMEDIATE), and have that > intermediate target have the dependencies. This is from version 3.74.3 > and we require 3.80 already, so this is fine. I think this is too artificial. Right now I just make the two generated .h files depend on the generated .c file, which works since they are all generated together or none of them are generated. That seems simple enough and more self-documenting to me. > >> +EXTRA_HEADERS += rs6000-vecdefines.h >> +rs6000-vecdefines.h : rs6000-builtins.c > No space before the colon please. Oopsie. Thanks! Bill > > > Segher
Hi! On Mon, Jul 26, 2021 at 10:26:25PM -0500, Bill Schmidt wrote: > On 7/21/21 1:58 PM, Segher Boessenkool wrote: > >On Thu, Jun 17, 2021 at 10:19:07AM -0500, Bill Schmidt wrote: > >>+# TODO: Whenever GNU make 4.3 is the minimum required, we should use > >>+# grouped targets on this: > >That may be quite a while still. GNU make is the foundation of > >everything, so we cannot require very new versions of it ever. > > > >In the meantime, you can make all these targets depend on an > >intermediate target (that you mark with .INTERMEDIATE), and have that > >intermediate target have the dependencies. This is from version 3.74.3 > >and we require 3.80 already, so this is fine. > I think this is too artificial. It is the generic solution. > Right now I just make the two generated > .h files depend on the generated .c file, which works since they are all > generated together or none of them are generated. That seems simple > enough and more self-documenting to me. It only works if you make sure the updates to the .c file are always seen by make only after the updates to the .h files are visible. The window in which it can race is small, but you have to make it empty :-( If you can guarantee that (ordering the fclose()s appropriately is enough I believe -- and add a comment for that :-) ) this should be fine, sure. Segher
Hi Segher, On 7/27/21 9:23 AM, Segher Boessenkool wrote: > On Mon, Jul 26, 2021 at 10:26:25PM -0500, Bill Schmidt wrote: >> Right now I just make the two generated >> .h files depend on the generated .c file, which works since they are all >> generated together or none of them are generated. That seems simple >> enough and more self-documenting to me. > It only works if you make sure the updates to the .c file are always > seen by make only after the updates to the .h files are visible. The > window in which it can race is small, but you have to make it empty :-( > > If you can guarantee that (ordering the fclose()s appropriately is > enough I believe -- and add a comment for that :-) ) this should be > fine, sure. > Good point. I have a small race because rs6000-builtins.c isn't closed after rs6000-vecdefines.h. I'll repair that as part of this patch and add commentary. Thanks again for the review! Bill
diff --git a/gcc/config.gcc b/gcc/config.gcc index 1be8d96f5e5..9ef4f7ffc12 100644 --- a/gcc/config.gcc +++ b/gcc/config.gcc @@ -475,6 +475,7 @@ powerpc*-*-*) cpu_type=rs6000 extra_objs="rs6000-string.o rs6000-p8swap.o rs6000-logue.o" extra_objs="${extra_objs} rs6000-call.o rs6000-pcrel-opt.o" + extra_objs="${extra_objs} rs6000-builtins.o rs6000-c.o" extra_headers="ppc-asm.h altivec.h htmintrin.h htmxlintrin.h" extra_headers="${extra_headers} bmi2intrin.h bmiintrin.h" extra_headers="${extra_headers} xmmintrin.h mm_malloc.h emmintrin.h" diff --git a/gcc/config/rs6000/t-rs6000 b/gcc/config/rs6000/t-rs6000 index 44f7ffb35fe..f8acc4ad90f 100644 --- a/gcc/config/rs6000/t-rs6000 +++ b/gcc/config/rs6000/t-rs6000 @@ -27,10 +27,6 @@ rs6000-pcrel-opt.o: $(srcdir)/config/rs6000/rs6000-pcrel-opt.c $(COMPILE) $< $(POSTCOMPILE) -rs6000-c.o: $(srcdir)/config/rs6000/rs6000-c.c - $(COMPILE) $< - $(POSTCOMPILE) - rs6000-string.o: $(srcdir)/config/rs6000/rs6000-string.c $(COMPILE) $< $(POSTCOMPILE) @@ -47,7 +43,45 @@ rs6000-logue.o: $(srcdir)/config/rs6000/rs6000-logue.c $(COMPILE) $< $(POSTCOMPILE) -rs6000-call.o: $(srcdir)/config/rs6000/rs6000-call.c +rs6000-gen-builtins.o: $(srcdir)/config/rs6000/rs6000-gen-builtins.c + $(COMPILE) $(CXXFLAGS) $< + $(POSTCOMPILE) + +rbtree.o: $(srcdir)/config/rs6000/rbtree.c + $(COMPILE) $< + $(POSTCOMPILE) + +rs6000-gen-builtins: rs6000-gen-builtins.o rbtree.o + $(LINKER_FOR_BUILD) $(BUILD_LINKERFLAGS) $(BUILD_LDFLAGS) -o $@ \ + $(filter-out $(BUILD_LIBDEPS), $^) $(BUILD_LIBS) + +# TODO: Whenever GNU make 4.3 is the minimum required, we should use +# grouped targets on this: +# rs6000-builtins.c rs6000-builtins.h rs6000-vecdefines.h &: <deps> +# <recipe> +rs6000-builtins.c: rs6000-gen-builtins \ + $(srcdir)/config/rs6000/rs6000-builtin-new.def \ + $(srcdir)/config/rs6000/rs6000-overload.def + ./rs6000-gen-builtins $(srcdir)/config/rs6000/rs6000-builtin-new.def \ + $(srcdir)/config/rs6000/rs6000-overload.def rs6000-builtins.h \ + rs6000-builtins.c rs6000-vecdefines.h + +rs6000-builtins.h: rs6000-builtins.c + +rs6000.o: rs6000-builtins.h + +EXTRA_HEADERS += rs6000-vecdefines.h +rs6000-vecdefines.h : rs6000-builtins.c + +rs6000-builtins.o: rs6000-builtins.c + $(COMPILE) $< + $(POSTCOMPILE) + +rs6000-call.o: $(srcdir)/config/rs6000/rs6000-call.c rs6000-builtins.h + $(COMPILE) $< + $(POSTCOMPILE) + +rs6000-c.o: $(srcdir)/config/rs6000/rs6000-c.c rs6000-builtins.h $(COMPILE) $< $(POSTCOMPILE)