[2/3] build: Add libgrust as compilation modules

Message ID 20230920120311.14892-3-arthur.cohen@embecosm.com
State Accepted
Headers
Series [1/3] librust: Add libproc_macro and build system |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

Arthur Cohen Sept. 20, 2023, 11:59 a.m. UTC
  From: Pierre-Emmanuel Patry <pierre-emmanuel.patry@embecosm.com>

Define the libgrust directory as a host compilation module as well as
for targets.

ChangeLog:

	* Makefile.def: Add libgrust as host & target module.
	* configure.ac: Add libgrust to host tools list.

gcc/rust/ChangeLog:

	* config-lang.in: Add libgrust as a target module for the rust
	language.

Signed-off-by: Pierre-Emmanuel Patry <pierre-emmanuel.patry@embecosm.com>
---
 Makefile.def            | 2 ++
 configure.ac            | 3 ++-
 gcc/rust/config-lang.in | 2 ++
 3 files changed, 6 insertions(+), 1 deletion(-)
  

Comments

Richard Biener Sept. 26, 2023, 11:42 a.m. UTC | #1
On Wed, Sep 20, 2023 at 2:04 PM Arthur Cohen <arthur.cohen@embecosm.com> wrote:
>
> From: Pierre-Emmanuel Patry <pierre-emmanuel.patry@embecosm.com>
>
> Define the libgrust directory as a host compilation module as well as
> for targets.

OK if you tested this doesn't break build when rust is enabled on trunk
(and doesn't build libgrust if not).

Richard.

> ChangeLog:
>
>         * Makefile.def: Add libgrust as host & target module.
>         * configure.ac: Add libgrust to host tools list.
>
> gcc/rust/ChangeLog:
>
>         * config-lang.in: Add libgrust as a target module for the rust
>         language.
>
> Signed-off-by: Pierre-Emmanuel Patry <pierre-emmanuel.patry@embecosm.com>
> ---
>  Makefile.def            | 2 ++
>  configure.ac            | 3 ++-
>  gcc/rust/config-lang.in | 2 ++
>  3 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/Makefile.def b/Makefile.def
> index 870150183b9..3df3fc18d14 100644
> --- a/Makefile.def
> +++ b/Makefile.def
> @@ -149,6 +149,7 @@ host_modules= { module= libcc1; extra_configure_flags=--enable-shared; };
>  host_modules= { module= gotools; };
>  host_modules= { module= libctf; bootstrap=true; };
>  host_modules= { module= libsframe; bootstrap=true; };
> +host_modules= { module= libgrust; };
>
>  target_modules = { module= libstdc++-v3;
>                    bootstrap=true;
> @@ -192,6 +193,7 @@ target_modules = { module= libgm2; lib_path=.libs; };
>  target_modules = { module= libgomp; bootstrap= true; lib_path=.libs; };
>  target_modules = { module= libitm; lib_path=.libs; };
>  target_modules = { module= libatomic; bootstrap=true; lib_path=.libs; };
> +target_modules = { module= libgrust; };
>
>  // These are (some of) the make targets to be done in each subdirectory.
>  // Not all; these are the ones which don't have special options.
> diff --git a/configure.ac b/configure.ac
> index 1d16530140a..036e5945905 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -133,7 +133,7 @@ build_tools="build-texinfo build-flex build-bison build-m4 build-fixincludes"
>
>  # these libraries are used by various programs built for the host environment
>  #f
> -host_libs="intl libiberty opcodes bfd readline tcl tk itcl libgui zlib libbacktrace libcpp libcody libdecnumber gmp mpfr mpc isl libiconv libctf libsframe"
> +host_libs="intl libiberty opcodes bfd readline tcl tk itcl libgui zlib libbacktrace libcpp libcody libdecnumber gmp mpfr mpc isl libiconv libctf libsframe libgrust "
>
>  # these tools are built for the host environment
>  # Note, the powerpc-eabi build depends on sim occurring before gdb in order to
> @@ -164,6 +164,7 @@ target_libraries="target-libgcc \
>                 target-libada \
>                 target-libgm2 \
>                 target-libgo \
> +               target-libgrust \
>                 target-libphobos \
>                 target-zlib"
>
> diff --git a/gcc/rust/config-lang.in b/gcc/rust/config-lang.in
> index aac66c9b962..8f071dcb0bf 100644
> --- a/gcc/rust/config-lang.in
> +++ b/gcc/rust/config-lang.in
> @@ -29,4 +29,6 @@ compilers="rust1\$(exeext)"
>
>  build_by_default="no"
>
> +target_libs="target-libffi target-libbacktrace target-libgrust"
> +
>  gtfiles="\$(srcdir)/rust/rust-lang.cc"
> --
> 2.42.0
>
  
Thomas Schwinge Sept. 26, 2023, 10:25 p.m. UTC | #2
Hi!

On 2023-09-20T13:59:53+0200, Arthur Cohen <arthur.cohen@embecosm.com> wrote:
> From: Pierre-Emmanuel Patry <pierre-emmanuel.patry@embecosm.com>
>
> Define the libgrust directory as a host compilation module as well as
> for targets.

> --- a/Makefile.def
> +++ b/Makefile.def
> @@ -149,6 +149,7 @@ host_modules= { module= libcc1; extra_configure_flags=--enable-shared; };
>  host_modules= { module= gotools; };
>  host_modules= { module= libctf; bootstrap=true; };
>  host_modules= { module= libsframe; bootstrap=true; };
> +host_modules= { module= libgrust; };
>
>  target_modules = { module= libstdc++-v3;
>                  bootstrap=true;
> @@ -192,6 +193,7 @@ target_modules = { module= libgm2; lib_path=.libs; };
>  target_modules = { module= libgomp; bootstrap= true; lib_path=.libs; };
>  target_modules = { module= libitm; lib_path=.libs; };
>  target_modules = { module= libatomic; bootstrap=true; lib_path=.libs; };
> +target_modules = { module= libgrust; };
>
>  // These are (some of) the make targets to be done in each subdirectory.
>  // Not all; these are the ones which don't have special options.

Maybe just I am confused, but to make sure that the build doesn't break
for different GCC configurations, don't we also directly need to
incorporate here a few GCC/Rust master branch follow-on commits, like:

  - commit 171ea4e2b3e202067c50f9c206974fbe1da691c0 "fixup: Fix bootstrap build"
  - commit 61cbe201029658c32e5c360823b9a1a17d21b03c "fixup: Fix missing build dependency"
  - commit 6a8b207b9ef7f9038e0cae7766117428783825d8 "libgrust: Add dependency to libstdc++"

(Not sure if all of these are necessary and/or if that's the complete
list; haven't looked up the corresponding GCC/Rust GitHub PRs.)

> --- a/gcc/rust/config-lang.in
> +++ b/gcc/rust/config-lang.in

> +target_libs="target-libffi target-libbacktrace target-libgrust"

Please don't add back 'target-libffi' and 'target-libbacktrace' here;
just 'target-libgrust'.  (As is present in GCC/Rust master branch, and
per commit 7411eca498beb13729cc2acec77e68250940aa81
"Rust: Don't depend on unused 'target-libffi', 'target-libbacktrace'".)


Grüße
 Thomas
-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955
  
Thomas Schwinge Oct. 27, 2023, 8:41 p.m. UTC | #3
Hi!

To close the loop here:

On 2023-09-27T00:25:16+0200, I wrote:
> On 2023-09-20T13:59:53+0200, Arthur Cohen <arthur.cohen@embecosm.com> wrote:
>> From: Pierre-Emmanuel Patry <pierre-emmanuel.patry@embecosm.com>
>>
>> Define the libgrust directory as a host compilation module as well as
>> for targets.
>
>> --- a/Makefile.def
>> +++ b/Makefile.def
>> @@ -149,6 +149,7 @@ host_modules= { module= libcc1; extra_configure_flags=--enable-shared; };
>>  host_modules= { module= gotools; };
>>  host_modules= { module= libctf; bootstrap=true; };
>>  host_modules= { module= libsframe; bootstrap=true; };
>> +host_modules= { module= libgrust; };
>>
>>  target_modules = { module= libstdc++-v3;
>>                  bootstrap=true;
>> @@ -192,6 +193,7 @@ target_modules = { module= libgm2; lib_path=.libs; };
>>  target_modules = { module= libgomp; bootstrap= true; lib_path=.libs; };
>>  target_modules = { module= libitm; lib_path=.libs; };
>>  target_modules = { module= libatomic; bootstrap=true; lib_path=.libs; };
>> +target_modules = { module= libgrust; };
>>
>>  // These are (some of) the make targets to be done in each subdirectory.
>>  // Not all; these are the ones which don't have special options.
>
> Maybe just I am confused, but to make sure that the build doesn't break
> for different GCC configurations

Indeed, as discussed in
<https://inbox.sourceware.org/875y2s170t.fsf@euler.schwinge.homeip.net>
"[PATCH v2 2/4] libgrust: Add libproc_macro and build system".

> don't we also directly need to
> incorporate here a few GCC/Rust master branch follow-on commits, like:
>
>   - commit 171ea4e2b3e202067c50f9c206974fbe1da691c0 "fixup: Fix bootstrap build"
>   - commit 61cbe201029658c32e5c360823b9a1a17d21b03c "fixup: Fix missing build dependency"

I've not yet run into the need for these two.  Let's please leave these
out of the upstream submission for now, until we understand what exactly
these are necessary for.

However:

>   - commit 6a8b207b9ef7f9038e0cae7766117428783825d8 "libgrust: Add dependency to libstdc++"

... this one definitely is necessary right now; see discussion in
<https://inbox.sourceware.org/8734xv24dd.fsf@euler.schwinge.homeip.net>
"Disable target libgrust if we're not building target libstdc++".


And:

> (Not sure if all of these are necessary and/or if that's the complete
> list; haven't looked up the corresponding GCC/Rust GitHub PRs.)
>
>> --- a/gcc/rust/config-lang.in
>> +++ b/gcc/rust/config-lang.in
>
>> +target_libs="target-libffi target-libbacktrace target-libgrust"
>
> Please don't add back 'target-libffi' and 'target-libbacktrace' here;
> just 'target-libgrust'.  (As is present in GCC/Rust master branch, and
> per commit 7411eca498beb13729cc2acec77e68250940aa81
> "Rust: Don't depend on unused 'target-libffi', 'target-libbacktrace'".)

... that change is necessary, too.


Grüße
 Thomas
-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955
  
Arthur Cohen Nov. 21, 2023, 3:20 p.m. UTC | #4
Hi Thomas!

A newer version of the library has been force-pushed to the branch 
`libgrust-v2/to-submit`.

On 11/20/23 15:55, Thomas Schwinge wrote:
> Hi!
> 
> Arthur and Pierre-Emmanuel have prepared a GCC/Rust libgrust-v2/to-submit
> branch: <https://github.com/Rust-GCC/gccrs/tree/libgrust-v2/to-submit>.
> In that one, most of the issues raised have been addressed, and which
> I've now successfully "tested" in my different GCC configurations,
> requiring just one additional change (see end of this email).  I'm using
> "tested" in quotes here, as libgrust currently is still missing its
> eventual content, and still is without actual users, so we may still be
> up for surprises later on.  ;-)
> 
> On 2023-10-27T22:41:52+0200, I wrote:
>> On 2023-09-27T00:25:16+0200, I wrote:
>>> don't we also directly need to
>>> incorporate here a few GCC/Rust master branch follow-on commits, like:
>>>
>>>    - commit 171ea4e2b3e202067c50f9c206974fbe1da691c0 "fixup: Fix bootstrap build"
>>>    - commit 61cbe201029658c32e5c360823b9a1a17d21b03c "fixup: Fix missing build dependency"
>>
>> I've not yet run into the need for these two.  Let's please leave these
>> out of the upstream submission for now, until we understand what exactly
>> these are necessary for.
> 
> (Still the same.)

Do you mean that we should remove the content of these commits from the 
submission? If so, I believe it's now done.

> 
>> However:
>>
>>>    - commit 6a8b207b9ef7f9038e0cae7766117428783825d8 "libgrust: Add dependency to libstdc++"
>>
>> ... this one definitely is necessary right now; see discussion in
>> <https://inbox.sourceware.org/8734xv24dd.fsf@euler.schwinge.homeip.net>
>> "Disable target libgrust if we're not building target libstdc++".
> 
> This one still isn't in the GCC/Rust libgrust-v2/to-submit branch -- but
> having now tested that branch, I'm now no longer seeing the respective
> build failure.  Isn't that change "libgrust: Add dependency to libstdc++"
> still necessary, conceptually?  (Maybe we're just lucky, currently?)
> I'll be sure to re-test in my different GCC configurations once libgrust
> gains actual content and use.  (..., which might then re-expose the
> original problem?)

This commit was integrated into another one:

fb31093105e build: Add libgrust as compilation modules

(on libgrust-v2/to-submit as of 2 minutes ago)

> 
>> And:
>>
>>> (Not sure if all of these are necessary and/or if that's the complete
>>> list; haven't looked up the corresponding GCC/Rust GitHub PRs.)
>>>
>>>> --- a/gcc/rust/config-lang.in
>>>> +++ b/gcc/rust/config-lang.in
>>>
>>>> +target_libs="target-libffi target-libbacktrace target-libgrust"
>>>
>>> Please don't add back 'target-libffi' and 'target-libbacktrace' here;
>>> just 'target-libgrust'.  (As is present in GCC/Rust master branch, and
>>> per commit 7411eca498beb13729cc2acec77e68250940aa81
>>> "Rust: Don't depend on unused 'target-libffi', 'target-libbacktrace'".)
>>
>> ... that change is necessary, too.
> 
> That's still unchanged in the GCC/Rust libgrust-v2/to-submit branch;
> please apply to 'gcc/rust/config-lang.in':
> 
>      -target_libs="target-libffi target-libbacktrace target-libgrust"
>      +target_libs=target-libgrust
> 
> Then, still should re-order the commits so that (re)generation of
> auto-generated files comes before use of libgrust (so that later
> bisection doesn't break), and move the 'contrib/gcc_update' update into
> the commit that adds the auto-generated files.

Do you mean that the regeneration should happen before the commit adding 
the proc_macro library? Or that when we keep going and adding more 
commits on top of this, we need to make sure the regeneration commit 
happens before any code starts using/depending on libgrust/?

And alright, we'll move the changes to contrib/gcc_update into the 
regeneration commit.

All the best, and thanks again for testing :)

Arthur

> 
> 
> Grüße
>   Thomas
> -----------------
> Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955
  
Thomas Schwinge Dec. 12, 2023, 9:39 a.m. UTC | #5
Hi Arthur, Pierre-Emmanuel!

On 2023-11-27T16:46:08+0100, I wrote:
> On 2023-11-21T16:20:22+0100, Arthur Cohen <arthur.cohen@embecosm.com> wrote:
>> On 11/20/23 15:55, Thomas Schwinge wrote:
>>> Arthur and Pierre-Emmanuel have prepared a GCC/Rust libgrust-v2/to-submit
>>> branch: <https://github.com/Rust-GCC/gccrs/tree/libgrust-v2/to-submit>.

..., and now updated once again.

>>> In that one, most of the issues raised have been addressed, and which
>>> I've now successfully "tested" in my different GCC configurations,
>>> requiring just one additional change (see end of this email).  I'm using
>>> "tested" in quotes here, as libgrust currently is still missing its
>>> eventual content, and still is without actual users, so we may still be
>>> up for surprises later on.  ;-)

>>> Then, still should re-order the commits [...]

> So I suggest:
>
>   - "libgrust: Add entry for maintainers and stub changelog file"
>   - "libgrust: Add libproc_macro and build system"
>     ... plus 'autoreconf' in 'libgrust/' folded in.
>     ... plus 'contrib/gcc_update' update moved here.
>   - "build: Add libgrust as compilation modules"
>     ... plus "Disable target libgrust if missing libstdc++" folded in.
>     ... plus 'autoreconf' and 'autogen'in '/' folded in.
>   - "Regenerate build files" then evaporates.

That's now been done, thanks.

Rebasing onto current master branch, there's a minor (textual) conflict
in top-level 'configure.ac:host_libs': 'intl' replaced by 'gettext', and
top-level 'configure' plus 'gcc/configure' have to be re-generated (the
latter for some unrelated changes in line numbers).  Otherwise, those
initial libgrust changes are now in the form that I thought they should
be in -- so I suggest you fix that up (I can quickly have a look again,
if you like), and then you do the "scary" 'git push' ;-) -- and then:

>> All the best, and thanks again for testing :)
>
> :-) So I hope I've not missed any major issues...

..., we wait and see.  :-)


Grüße
 Thomas
-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955
  
Thomas Schwinge Dec. 12, 2023, 3:25 p.m. UTC | #6
Hi Arthur, Pierre-Emmanuel!

On 2023-12-12T10:39:50+0100, I wrote:
> On 2023-11-27T16:46:08+0100, I wrote:
>> On 2023-11-21T16:20:22+0100, Arthur Cohen <arthur.cohen@embecosm.com> wrote:
>>> On 11/20/23 15:55, Thomas Schwinge wrote:
>>>> Arthur and Pierre-Emmanuel have prepared a GCC/Rust libgrust-v2/to-submit
>>>> branch: <https://github.com/Rust-GCC/gccrs/tree/libgrust-v2/to-submit>.

> Rebasing onto current master branch, there's a minor (textual) conflict
> in top-level 'configure.ac:host_libs': 'intl' replaced by 'gettext', and
> top-level 'configure' plus 'gcc/configure' have to be re-generated (the
> latter for some unrelated changes in line numbers).  Otherwise, those
> initial libgrust changes are now in the form that I thought they should
> be in -- so I suggest you fix that up (I can quickly have a look again,
> if you like)

I've noticed that you've fix that up (looks good), but I also noticed one
additional small item: into "build: Add libgrust as compilation modules",
you'll have to add the effect of top-level 'autogen Makefile.def' (that
is, regenerate the top-level 'Makefile.in').


Grüße
 Thomas


> and then you do the "scary" 'git push' ;-) -- and then:
>
>>> All the best, and thanks again for testing :)
>>
>> :-) So I hope I've not missed any major issues...
>
> ..., we wait and see.  :-)
>
>
> Grüße
>  Thomas
-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955
  

Patch

diff --git a/Makefile.def b/Makefile.def
index 870150183b9..3df3fc18d14 100644
--- a/Makefile.def
+++ b/Makefile.def
@@ -149,6 +149,7 @@  host_modules= { module= libcc1; extra_configure_flags=--enable-shared; };
 host_modules= { module= gotools; };
 host_modules= { module= libctf; bootstrap=true; };
 host_modules= { module= libsframe; bootstrap=true; };
+host_modules= { module= libgrust; };
 
 target_modules = { module= libstdc++-v3;
 		   bootstrap=true;
@@ -192,6 +193,7 @@  target_modules = { module= libgm2; lib_path=.libs; };
 target_modules = { module= libgomp; bootstrap= true; lib_path=.libs; };
 target_modules = { module= libitm; lib_path=.libs; };
 target_modules = { module= libatomic; bootstrap=true; lib_path=.libs; };
+target_modules = { module= libgrust; };
 
 // These are (some of) the make targets to be done in each subdirectory.
 // Not all; these are the ones which don't have special options.
diff --git a/configure.ac b/configure.ac
index 1d16530140a..036e5945905 100644
--- a/configure.ac
+++ b/configure.ac
@@ -133,7 +133,7 @@  build_tools="build-texinfo build-flex build-bison build-m4 build-fixincludes"
 
 # these libraries are used by various programs built for the host environment
 #f
-host_libs="intl libiberty opcodes bfd readline tcl tk itcl libgui zlib libbacktrace libcpp libcody libdecnumber gmp mpfr mpc isl libiconv libctf libsframe"
+host_libs="intl libiberty opcodes bfd readline tcl tk itcl libgui zlib libbacktrace libcpp libcody libdecnumber gmp mpfr mpc isl libiconv libctf libsframe libgrust "
 
 # these tools are built for the host environment
 # Note, the powerpc-eabi build depends on sim occurring before gdb in order to
@@ -164,6 +164,7 @@  target_libraries="target-libgcc \
 		target-libada \
 		target-libgm2 \
 		target-libgo \
+		target-libgrust \
 		target-libphobos \
 		target-zlib"
 
diff --git a/gcc/rust/config-lang.in b/gcc/rust/config-lang.in
index aac66c9b962..8f071dcb0bf 100644
--- a/gcc/rust/config-lang.in
+++ b/gcc/rust/config-lang.in
@@ -29,4 +29,6 @@  compilers="rust1\$(exeext)"
 
 build_by_default="no"
 
+target_libs="target-libffi target-libbacktrace target-libgrust"
+
 gtfiles="\$(srcdir)/rust/rust-lang.cc"