Re: [PATCH v2 2/4] libgrust: Add libproc_macro and build system

[Iain Sandoe]

> On 26 Oct 2023, at 09:21, Thomas Schwinge  wrote:
> 
> Hi!
> 
> First, I've pushed into GCC upstream Git branch devel/rust/libgrust-v2
> the "v2" libgrust changes as posted by Arthur, so that people can easily
> test this before it getting into Git master branch.
> 
> I'll myself later try this for GCN and nvptx targets -- in their current
> form where they don't support C++ (standard library), and in my hacky WIP
> trees where C++ (standard library) is supported to some extent.  (This
> should, roughly, match C++ functionality (not) provided by a number of
> other GCC "embedded" targets.)

on Darwin, it works for later systems without multilibs, but fails to build 
multilibs.

—— so….

With the patch below bootstrap suceeded on x86_64-darwin17 and produced a 
correct
architecture multilib.  Of course, there is no way to test this at the moment - 
I’d suggest
that the next step might be something small in functionality that can allow at 
least one
test to be wired up.

^^^ this is “lightly tested” of course, as I cycle through other versions of 
the OS will see
how it pans out.

Do you want me to make a PR with this change against upstream?

Iain



0001-libgrust-enable-multilib.patch
Description: Binary data

> 
> 
> Then:
> 
> On 2023-10-25T13:06:46+0200, Arthur Cohen  wrote:
>> From: Pierre-Emmanuel Patry 
>> 
>> Add some dummy files in libproc_macro along with its build system.
> 
> I've not reviewed the build system in detail, just had a very quick look.
> 
> Three instances of 'librust'; should be 'libgrust':
> 
>configure.ac:AC_INIT([libgrust], version-unused,,librust)
> 
>configure.ac:AC_MSG_NOTICE([librust has been configured.])
> 
>Makefile.am:"TARGET_LIB_PATH_librust=$(TARGET_LIB_PATH_librust)" \
> 
> Compared to libgomp (which I'm reasonably familiar with), I found missing
> in 'libgrust' at 'configure'-level:
> 
>  --enable-multilib   build many library versions (default)
> 
>  --disable-werrordisable building with -Werror
> 
>  --enable-symvers=STYLE  enables symbol versioning of the shared library
>  [default=yes]
> 
>  --enable-cetenable Intel CET in target libraries 
> [default=auto]
> 
>  --with-gcc-major-version-only
>  use only GCC major number in filesystem paths
> 
> I can't tell off-hand whether all these are important, however.
> 
> Additionally, the new one that's being discussed in
> 
> 'Update libgrust for upstream GCC commit 
> 6a6d3817afa02bbcd2388c8e005da6faf88932f1 "Config,Darwin: Allow for 
> configuring Darwin to use embedded runpath"'.
> 
> 
> Grüße
> Thomas
> 
> 
>> libgrust/Changelog:
>> 
>>  * Makefile.am: New file.
>>  * configure.ac: New file.
>>  * libproc_macro/Makefile.am: New file.
>>  * libproc_macro/proc_macro.cc: New file.
>>  * libproc_macro/proc_macro.h: New file.
>> 
>> Signed-off-by: Pierre-Emmanuel Patry 
>> ---
>> libgrust/Makefile.am |  68 
>> libgrust/configure.ac| 113 +++
>> libgrust/libproc_macro/Makefile.am   |  58 ++
>> libgrust/libproc_macro/proc_macro.cc |   7 ++
>> libgrust/libproc_macro/proc_macro.h  |   7 ++
>> 5 files changed, 253 insertions(+)
>> create mode 100644 libgrust/Makefile.am
>> create mode 100644 libgrust/configure.ac
>> create mode 100644 libgrust/libproc_macro/Makefile.am
>> create mode 100644 libgrust/libproc_macro/proc_macro.cc
>> create mode 100644 libgrust/libproc_macro/proc_macro.h
>> 
>> diff --git a/libgrust/Makefile.am b/libgrust/Makefile.am
>> new file mode 100644
>> index 000..8e5274922c5
>> --- /dev/null
>> +++ b/libgrust/Makefile.am
>> @@ -0,0 +1,68 @@
>> +AUTOMAKE_OPTIONS = 1.8 foreign
>> +
>> +SUFFIXES = .c .rs .def .o .lo .a
>> +
>> +ACLOCAL_AMFLAGS = -I . -I .. -I ../config
>> +
>> +AM_CFLAGS = -I $(srcdir)/../libgcc -I $(MULTIBUILDTOP)../../gcc/include
>> +
>> +TOP_GCCDIR := $(shell cd $(top_srcdir) && cd .. && pwd)
>> +
>> +GCC_DIR = $(TOP_GCCDIR)/gcc
>> +RUST_SRC = $(GCC_DIR)/rust
>> +
>> +toolexeclibdir=@toolexeclibdir@
>> +toolexecdir=@toolexecdir@
>> +
>> +SUBDIRS = libproc_macro
>> +
>> +RUST_BUILDDIR := $(shell pwd)
>> +
>> +# Work around what appears to be a GNU make bug handling MAKEFLAGS
>> +# values defined in terms of make variables, as is the case for CC and
>> +# friends when we are called from the top level Makefile.
>> +AM_MAKEFLAGS = \
>> +"GCC_DIR=$(GCC_DIR)" \
>> +"RUST_SRC=$(RUST_SRC)" \
>> + "AR_FLAGS=$(AR_FLAGS)" \
>> + "CC_FOR_BUILD=$(CC_FOR_BUILD)" \
>> + "CC_FOR_TARGET=$(CC_FOR_TARGET)" \
>> + "RUST_FOR_TARGET=$(RUST_FOR_TARGET)" \
>> + "CFLAGS=$(CFLAGS)" \
>> + "CXXFLAGS=$(CXXFLAGS)" \
>> + "CFLAGS_FOR_BUILD=$(CFLAGS_FOR_BUILD)" \
>> + "CFLAGS_FOR_TARGET=$(CFLAGS_FOR_TARGET)" \
>> + "INSTALL=$(INSTALL)" \
>> + "INSTALL_DATA=$(INSTALL_DATA)" \
>> + "IN

Re: [COMMITTED] rust_debug: Cast size_t values to unsigned long before printing.

[Iain Sandoe]

Hi Arthur,

> On 18 Jan 2024, at 10:30, Arthur Cohen  wrote:

> On 1/18/24 10:13, Rainer Orth wrote:
>> Arthur Cohen  writes:
>>> Using %lu to format size_t values breaks 32 bit targets, and %zu is not
>>> supported by one of the hosts GCC aims to support - HPUX
>> But we do have uses of %zu in gcc/rust already!
>>> diff --git a/gcc/rust/expand/rust-proc-macro.cc 
>>> b/gcc/rust/expand/rust-proc-macro.cc
>>> index e8618485b71..09680733e98 100644
>>> --- a/gcc/rust/expand/rust-proc-macro.cc
>>> +++ b/gcc/rust/expand/rust-proc-macro.cc
>>> @@ -171,7 +171,7 @@ load_macros (std::string path)
>>>if (array == nullptr)
>>>  return {};
>>>  -  rust_debug ("Found %lu procedural macros", array->length);
>>> +  rust_debug ("Found %lu procedural macros", (unsigned long) 
>>> array->length);
>> Not the best way either: array->length is std::uint64_t, so the format
>> should use
>> ... %" PRIu64 " procedural...
>> instead.
>> I've attached my patch to PR rust/113461.
> 
> Yes, I was talking about this on IRC the other day - if we do run in a 
> situation where we have more than UINT32_MAX procedural macros in memory we 
> have big issues. These debug prints will probably end up getting removed soon 
> as they clutter the output a lot for little information.
> 
> I don't mind doing it the right way for our regular prints, but we have not 
> been using PRIu64 in our codebase so far, so I'd rather change all those 
> incriminating format specifiers at once later down the line - this patch was 
> pushed so that 32bit targets could bootstrap the Rust frontend for now.

For the sake of completeness, the issue does not just affect 32b hosts;  If a 
64b host chooses (as Darwin does, so that 32b and 64b targets have the same 
representation) to make uint64_t “unsigned long long int”, then %lu breaks 
there too.
thanks
Iain

Re: [PATCH] build: Check for cargo when building rust language

[Iain Sandoe]

Hi Arthur,

> On 9 Apr 2024, at 11:40, Arthur Cohen  wrote:

> On 4/9/24 09:47, John Paul Adrian Glaubitz wrote:
>> Hello,
>> On Mon, 2024-04-08 at 18:33 +0200, pierre-emmanuel.pa...@embecosm.com wrote:
>>> The rust frontend requires cargo to build some of it's components,
>>> it's presence was not checked during configuration.
>> Isn't this creating a hen-and-egg problem? How am I supposed to build a Rust
>> compiler for a target which is not supported by rustc (yet) when gccrs is
>> supposed to build-depend on cargo which requires rustc?
>> Adrian
> 
> Quick reminder in case you haven't seen our Request for Comments on the main 
> ML that this is only a temporary solution. Once gccrs can compile its 
> dependencies, we'll go through a more "classical" bootstrapping chain.


I don’t suppose there’s some way to make a “download prerequisites” action for 
this?

(I realise that the prerequisite might not be available for a given platform - 
but then the configure will then just fail to detect them and carry on).

At the least the build documentation requested should (ideally) try to lower 
the barrier to finding the deps and give reliable sources for them.

> rustc_codegen_gcc can probably already be used for building these 
> dependencies however, if you'd like to have a look at that.

Detailing the verious options would also be a helpful part of the build doc.

thanks
Iain

Re: [PATCH] build: Check for cargo when building rust language

[Iain Sandoe]

Hi Arthur,

> On 9 Apr 2024, at 13:01, Arthur Cohen  wrote:
> 

> On 4/9/24 10:55, Iain Sandoe wrote:
>> Hi Arthur,
>>> On 9 Apr 2024, at 11:40, Arthur Cohen  wrote:
>>> On 4/9/24 09:47, John Paul Adrian Glaubitz wrote:
>>>> Hello,
>>>> On Mon, 2024-04-08 at 18:33 +0200, pierre-emmanuel.pa...@embecosm.com 
>>>> wrote:
>>>>> The rust frontend requires cargo to build some of it's components,
>>>>> it's presence was not checked during configuration.
>>>> Isn't this creating a hen-and-egg problem? How am I supposed to build a 
>>>> Rust
>>>> compiler for a target which is not supported by rustc (yet) when gccrs is
>>>> supposed to build-depend on cargo which requires rustc?
>>>> Adrian
>>> 
>>> Quick reminder in case you haven't seen our Request for Comments on the 
>>> main ML that this is only a temporary solution. Once gccrs can compile its 
>>> dependencies, we'll go through a more "classical" bootstrapping chain.
>> I don’t suppose there’s some way to make a “download prerequisites” action 
>> for this?
> 
> Do you mean downloading cargo/Rust as a prerequisite? I don't believe this is 
> being done for GNAT/GDC, but I might be wrong.

No, you are quite correct, but the critical difference is that Ada and D both 
make use of earlier versions of GCC - so that (if one wished to be particular) 
it is possible to start with an earlier version of GCC and work forwards (in 
fact that’s what I’ve [I guess all of us] have done for D … and did a looong 
time ago for Ada).

The difference here is that we need to install an executable from somewhere 
else - and making that as simple and trustworthy as possible seems like a good 
move to encourage folks to build & test rust.

> If you mean the dependencies for our Rust components, those are currently 
> being vendored so that we're able to build them offline. I'll push the 
> commits soon.

OK.. I’m sorry to say this - but what’s actually needed is still a little fuzzy 
to me - but I am happy to wait to read the documentation patch and comment then.

thanks
Iain

> 
>> (I realise that the prerequisite might not be available for a given platform 
>> - but then the configure will then just fail to detect them and carry on).
>> At the least the build documentation requested should (ideally) try to lower 
>> the barrier to finding the deps and give reliable sources for them.
>>> rustc_codegen_gcc can probably already be used for building these 
>>> dependencies however, if you'd like to have a look at that.
>> Detailing the verious options would also be a helpful part of the build doc.
>> thanks
>> Iain
> 
> Best,
> 
> Arthur