[PATCH] Better make rules for IPA objects

[PATCH] Better make rules for IPA objects

[Alan Hayward]

This patch strengthens the rule for compiling arch/ and common/ files
into IPA.

In the existing code, "foo-ipa.o" will try to match:
foo-generated.c
foo-ipa.c
gdbserver/foo.c
common/foo.c
arch/foo.c

If this potentially matched multiple files, then the first is matched.

This patch changes the IPA rules so that files in arch/ and common/ are
explicitly listed using the directory name.

A future patch could be added to remove the ambiguity from the first three
matches. I'm not planning on making that change.

This changed is required as part of moving aarch64 to use flexible target
descriptors.

Alan.

2017-11-07  Alan Hayward  <alan.hayward@arm.com>

gdbserver:
	* Makefile.in: Update arch and common rules.
	* configure.srv: Explicitly mark arch/ and common/ files.

diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
index 8e73563b103f720ddd5e77607c3190a2959903f5..1bd4cf93cce192f060c362665fd5df9f4c323f24 100644
--- a/gdb/gdbserver/Makefile.in
+++ b/gdb/gdbserver/Makefile.in
@@ -563,11 +563,11 @@ arch/%.o: ../arch/%.c
 	$(IPAGENT_COMPILE) $<
 	$(POSTCOMPILE)

-%-ipa.o: ../common/%.c
+common/%-ipa.o: ../common/%.c
 	$(IPAGENT_COMPILE) $<
 	$(POSTCOMPILE)

-%-ipa.o: ../arch/%.c
+arch/%-ipa.o: ../arch/%.c
 	$(IPAGENT_COMPILE) $<
 	$(POSTCOMPILE)

diff --git a/gdb/gdbserver/configure.srv b/gdb/gdbserver/configure.srv
index 515c6dc8b3e57574286149ebdca37da149218a35..82c3dc237c3fb6baeb2a72cd9ed66d8c7556d72a 100644
--- a/gdb/gdbserver/configure.srv
+++ b/gdb/gdbserver/configure.srv
@@ -133,7 +133,7 @@ case "${target}" in
 			srv_linux_thread_db=yes
 			srv_linux_btrace=yes
 			ipa_obj="linux-i386-ipa.o linux-x86-tdesc-ipa.o"
-			ipa_obj="${ipa_obj} i386-ipa.o"
+			ipa_obj="${ipa_obj} arch/i386-ipa.o"
 			;;
   i[34567]86-*-lynxos*)	srv_regobj=""
 			srv_tgtobj="lynx-low.o lynx-i386-low.o fork-child.o fork-inferior.o"
@@ -383,7 +383,7 @@ case "${target}" in
 			srv_linux_thread_db=yes
 			srv_linux_btrace=yes
 			ipa_obj="linux-amd64-ipa.o linux-x86-tdesc-ipa.o"
-			ipa_obj="${ipa_obj} amd64-ipa.o"
+			ipa_obj="${ipa_obj} arch/amd64-ipa.o"
 			;;
   x86_64-*-mingw*)	srv_regobj=""
 			srv_tgtobj="x86-low.o x86-dregs.o i387-fp.o win32-low.o win32-i386-low.o”

Re: [PATCH] Better make rules for IPA objects

[Alan Hayward]

Ping.

> On 7 Nov 2017, at 10:18, Alan Hayward <Alan.Hayward@arm.com> wrote:
> 
> This patch strengthens the rule for compiling arch/ and common/ files
> into IPA.
> 
> In the existing code, "foo-ipa.o" will try to match:
> foo-generated.c
> foo-ipa.c
> gdbserver/foo.c
> common/foo.c
> arch/foo.c
> 
> If this potentially matched multiple files, then the first is matched.
> 
> This patch changes the IPA rules so that files in arch/ and common/ are
> explicitly listed using the directory name.
> 
> A future patch could be added to remove the ambiguity from the first three
> matches. I'm not planning on making that change.
> 
> This changed is required as part of moving aarch64 to use flexible target
> descriptors.
> 
> Alan.
> 
> 2017-11-07  Alan Hayward  <alan.hayward@arm.com>
> 
> gdbserver:
> 	* Makefile.in: Update arch and common rules.
> 	* configure.srv: Explicitly mark arch/ and common/ files.
> 
> diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
> index 8e73563b103f720ddd5e77607c3190a2959903f5..1bd4cf93cce192f060c362665fd5df9f4c323f24 100644
> --- a/gdb/gdbserver/Makefile.in
> +++ b/gdb/gdbserver/Makefile.in
> @@ -563,11 +563,11 @@ arch/%.o: ../arch/%.c
> 	$(IPAGENT_COMPILE) $<
> 	$(POSTCOMPILE)
> 
> -%-ipa.o: ../common/%.c
> +common/%-ipa.o: ../common/%.c
> 	$(IPAGENT_COMPILE) $<
> 	$(POSTCOMPILE)
> 
> -%-ipa.o: ../arch/%.c
> +arch/%-ipa.o: ../arch/%.c
> 	$(IPAGENT_COMPILE) $<
> 	$(POSTCOMPILE)
> 
> diff --git a/gdb/gdbserver/configure.srv b/gdb/gdbserver/configure.srv
> index 515c6dc8b3e57574286149ebdca37da149218a35..82c3dc237c3fb6baeb2a72cd9ed66d8c7556d72a 100644
> --- a/gdb/gdbserver/configure.srv
> +++ b/gdb/gdbserver/configure.srv
> @@ -133,7 +133,7 @@ case "${target}" in
> 			srv_linux_thread_db=yes
> 			srv_linux_btrace=yes
> 			ipa_obj="linux-i386-ipa.o linux-x86-tdesc-ipa.o"
> -			ipa_obj="${ipa_obj} i386-ipa.o"
> +			ipa_obj="${ipa_obj} arch/i386-ipa.o"
> 			;;
>  i[34567]86-*-lynxos*)	srv_regobj=""
> 			srv_tgtobj="lynx-low.o lynx-i386-low.o fork-child.o fork-inferior.o"
> @@ -383,7 +383,7 @@ case "${target}" in
> 			srv_linux_thread_db=yes
> 			srv_linux_btrace=yes
> 			ipa_obj="linux-amd64-ipa.o linux-x86-tdesc-ipa.o"
> -			ipa_obj="${ipa_obj} amd64-ipa.o"
> +			ipa_obj="${ipa_obj} arch/amd64-ipa.o"
> 			;;
>  x86_64-*-mingw*)	srv_regobj=""
> 			srv_tgtobj="x86-low.o x86-dregs.o i387-fp.o win32-low.o win32-i386-low.o”
> 
> 

Re: [PATCH] Better make rules for IPA objects

[Yao Qi]

Alan Hayward <Alan.Hayward@arm.com> writes:

> -%-ipa.o: ../common/%.c
> +common/%-ipa.o: ../common/%.c
>  	$(IPAGENT_COMPILE) $<
>  	$(POSTCOMPILE)

We don't have common/ directory in GDBserver build tree, so
common/%-ipa.o is useless.  Secondly, this "%-ipa.o: ../common/%.c"
is removed, common/*.c files can't be built for IPA.  Please remove this
change.

With your patch applied, fail to build libinproctrace.so on x86_64-linux.

g++ -std=gnu++11 -shared -fPIC -Wl,--soname=libinproctrace.so -Wl,--no-undefined -g -O2    -I. -I../../../binutils-gdb/gdb/gdbserver -I../../../binutils-gdb/gdb/gdbserver/../common -I../../../binutils-gdb/gdb/gdbserver/../regformats -I../../../binutils-gdb/gdb/gdbserver/.. -I../../../binutils-gdb/gdb/gdbserver/../../include -I../../../binutils-gdb/gdb/gdbserver/../gnulib/import -Ibuild-gnulib-gdbserver/import  -Wall -Wpointer-arith -Wno-unused -Wunused-value -Wunused-function -Wno-switch -Wno-char-subscripts -Wempty-body -Wunused-but-set-parameter -Wunused-but-set-variable -Wno-sign-compare -Wno-narrowing -Wno-error=maybe-uninitialized -Wformat-nonliteral -Werror -DGDBSERVER \
-static-libstdc++ -static-libgcc  -Wl,--dynamic-list=../../../binutils-gdb/gdb/gdbserver/proc-service.list -o libinproctrace.so ax-ipa.o common-utils-ipa.o errors-ipa.o format-ipa.o print-utils-ipa.o regcache-ipa.o remote-utils-ipa.o rsp-low-ipa.o tdesc-ipa.o tracepoint-ipa.o utils-ipa.o vec-ipa.o linux-amd64-ipa.o linux-x86-tdesc-ipa.o arch/amd64-ipa.o -ldl -pthread
g++: error: common-utils-ipa.o: No such file or directory
g++: error: errors-ipa.o: No such file or directory
g++: error: format-ipa.o: No such file or directory
g++: error: print-utils-ipa.o: No such file or directory
g++: error: rsp-low-ipa.o: No such file or directory
g++: error: vec-ipa.o: No such file or directory
Makefile:412: recipe for target 'libinproctrace.so' failed

This and (https://sourceware.org/ml/gdb-patches/2017-10/msg00888.html)
worry me more.  You miss something important in the development
workflow to make sure your patches posted are well-tested.

-- 
Yao (齐尧)

Re: [PATCH] Better make rules for IPA objects

[Alan Hayward]

> On 14 Nov 2017, at 11:14, Yao Qi <qiyaoltc@gmail.com> wrote:
> 
> Alan Hayward <Alan.Hayward@arm.com> writes:
> 
>> -%-ipa.o: ../common/%.c
>> +common/%-ipa.o: ../common/%.c
>> 	$(IPAGENT_COMPILE) $<
>> 	$(POSTCOMPILE)
> 
> We don't have common/ directory in GDBserver build tree, so
> common/%-ipa.o is useless.  Secondly, this "%-ipa.o: ../common/%.c"
> is removed, common/*.c files can't be built for IPA.  Please remove this
> change.
> 
> With your patch applied, fail to build libinproctrace.so on x86_64-linux.
> 
> g++ -std=gnu++11 -shared -fPIC -Wl,--soname=libinproctrace.so -Wl,--no-undefined -g -O2    -I. -I../../../binutils-gdb/gdb/gdbserver -I../../../binutils-gdb/gdb/gdbserver/../common -I../../../binutils-gdb/gdb/gdbserver/../regformats -I../../../binutils-gdb/gdb/gdbserver/.. -I../../../binutils-gdb/gdb/gdbserver/../../include -I../../../binutils-gdb/gdb/gdbserver/../gnulib/import -Ibuild-gnulib-gdbserver/import  -Wall -Wpointer-arith -Wno-unused -Wunused-value -Wunused-function -Wno-switch -Wno-char-subscripts -Wempty-body -Wunused-but-set-parameter -Wunused-but-set-variable -Wno-sign-compare -Wno-narrowing -Wno-error=maybe-uninitialized -Wformat-nonliteral -Werror -DGDBSERVER \
> -static-libstdc++ -static-libgcc  -Wl,--dynamic-list=../../../binutils-gdb/gdb/gdbserver/proc-service.list -o libinproctrace.so ax-ipa.o common-utils-ipa.o errors-ipa.o format-ipa.o print-utils-ipa.o regcache-ipa.o remote-utils-ipa.o rsp-low-ipa.o tdesc-ipa.o tracepoint-ipa.o utils-ipa.o vec-ipa.o linux-amd64-ipa.o linux-x86-tdesc-ipa.o arch/amd64-ipa.o -ldl -pthread
> g++: error: common-utils-ipa.o: No such file or directory
> g++: error: errors-ipa.o: No such file or directory
> g++: error: format-ipa.o: No such file or directory
> g++: error: print-utils-ipa.o: No such file or directory
> g++: error: rsp-low-ipa.o: No such file or directory
> g++: error: vec-ipa.o: No such file or directory
> Makefile:412: recipe for target 'libinproctrace.so’ failed
> 

The idea was that the new common rule was compiling the files from
gdbserver/../common/, e.g. gdbserver/../common/common-utils.c.
...Which I’ve now broken...

Ok, removed this change from the new version below.

> This and (https://sourceware.org/ml/gdb-patches/2017-10/msg00888.html)
> worry me more.  You miss something important in the development
> workflow to make sure your patches posted are well-tested.
> 

Apologies for this. Possibly I didn’t do a complete make clean here.
But it was glaringly obvious when I recompiled again.

Alan.

gdbserver:
	* Makefile.in: Update arch rules.
	* configure.srv: Explicitly mark arch/  files.


diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
index 8e73563b103f720ddd5e77607c3190a2959903f5..1d51819014270e48b6a670e3defa1dc449e3abf5 100644
--- a/gdb/gdbserver/Makefile.in
+++ b/gdb/gdbserver/Makefile.in
@@ -567,7 +567,7 @@ arch/%.o: ../arch/%.c
 	$(IPAGENT_COMPILE) $<
 	$(POSTCOMPILE)

-%-ipa.o: ../arch/%.c
+arch/%-ipa.o: ../arch/%.c
 	$(IPAGENT_COMPILE) $<
 	$(POSTCOMPILE)

diff --git a/gdb/gdbserver/configure.srv b/gdb/gdbserver/configure.srv
index 515c6dc8b3e57574286149ebdca37da149218a35..82c3dc237c3fb6baeb2a72cd9ed66d8c7556d72a 100644
--- a/gdb/gdbserver/configure.srv
+++ b/gdb/gdbserver/configure.srv
@@ -133,7 +133,7 @@ case "${target}" in
 			srv_linux_thread_db=yes
 			srv_linux_btrace=yes
 			ipa_obj="linux-i386-ipa.o linux-x86-tdesc-ipa.o"
-			ipa_obj="${ipa_obj} i386-ipa.o"
+			ipa_obj="${ipa_obj} arch/i386-ipa.o"
 			;;
   i[34567]86-*-lynxos*)	srv_regobj=""
 			srv_tgtobj="lynx-low.o lynx-i386-low.o fork-child.o fork-inferior.o"
@@ -383,7 +383,7 @@ case "${target}" in
 			srv_linux_thread_db=yes
 			srv_linux_btrace=yes
 			ipa_obj="linux-amd64-ipa.o linux-x86-tdesc-ipa.o"
-			ipa_obj="${ipa_obj} amd64-ipa.o"
+			ipa_obj="${ipa_obj} arch/amd64-ipa.o"
 			;;
   x86_64-*-mingw*)	srv_regobj=""
 			srv_tgtobj="x86-low.o x86-dregs.o i387-fp.o win32-low.o win32-i386-low.o"

Re: [PATCH] Better make rules for IPA objects

[Yao Qi]

Alan Hayward <Alan.Hayward@arm.com> writes:

> gdbserver:
> 	* Makefile.in: Update arch rules.
> 	* configure.srv: Explicitly mark arch/  files.

Patch is good to me.

-- 
Yao (齐尧)