Better make rules for IPA objects

Message ID 9FD839CC-67AD-43A8-B28F-11F78BB6BC94@arm.com
State New, archived
Headers

Commit Message

Alan Hayward Nov. 7, 2017, 10:40 a.m. UTC
  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.
  

Comments

Alan Hayward Nov. 14, 2017, 9:53 a.m. UTC | #1
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”

> 

>
  
Yao Qi Nov. 14, 2017, 11:14 a.m. UTC | #2
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.
  

Patch

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”