[23/55] rs6000: Incorporate new builtins code into the build machinery

Message ID 4f27468885fd84aba5f0dc2369a284f3cffa2b34.1623941441.git.wschmidt@linux.ibm.com
State New
Headers show
Series
  • Replace the Power target-specific builtin machinery
Related show

Commit Message

Andrew Pinski via Gcc-patches June 17, 2021, 3:19 p.m.
2021-06-07  Bill Schmidt  <wschmidt@linux.ibm.com>

gcc/
	* config.gcc (extra_objs): Include rs6000-builtins.o and
	rs6000-c.o.
	* 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.
---
 gcc/config.gcc             |  1 +
 gcc/config/rs6000/t-rs6000 | 44 +++++++++++++++++++++++++++++++++-----
 2 files changed, 40 insertions(+), 5 deletions(-)

-- 
2.27.0

Comments

Segher Boessenkool July 21, 2021, 6:58 p.m. | #1
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
Andrew Pinski via Gcc-patches July 27, 2021, 3:26 a.m. | #2
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
Segher Boessenkool July 27, 2021, 2:23 p.m. | #3
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
Andrew Pinski via Gcc-patches July 27, 2021, 5:38 p.m. | #4
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

Patch

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)