public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Thomas Schwinge <thomas@codesourcery.com>
To: Philip Herron <philip.herron@embecosm.com>, <gcc-patches@gcc.gnu.org>
Cc: <herron.philip@googlemail.com>,
	SimplyTheOther <simplytheother@gmail.com>
Subject: Re: [PATCH Rust front-end v1 2/4] Add Rust lang TargetHooks for i386 and x86_64
Date: Thu, 28 Jul 2022 12:38:06 +0200	[thread overview]
Message-ID: <878roda5f5.fsf@euler.schwinge.homeip.net> (raw)
In-Reply-To: <20220727134040.843750-3-philip.herron@embecosm.com>

[-- Attachment #1: Type: text/plain, Size: 4541 bytes --]

Hi!

On 2022-07-27T14:40:38+0100, "herron.philip--- via Gcc-patches" <gcc-patches@gcc.gnu.org> wrote:
> This patch introduces a new set of interfaces to define the target info as
> expected by the rust front-end. It takes advantage of the information
> within gcc/config/target directories which gets called by the front-end
> to populate rust front-end datastructures by calling into:
> builtin_rust_info. This patch has been isolated to find if we are
> approaching this in an idiomatic way and is compilable without the
> rust-front-end code.

I suppose the general approach may be fine, as is similarly implemented
by other languages' front ends in GCC.

> We have received many patches here which gives us the target hook info for
> most platforms

But this is all so much WIP and full of TODO notes, and has no test cases
at all!, that I still don't really see much value in keeping the current
implementations of 'TARGET_RUST_CPU_INFO', 'TARGET_RUST_OS_INFO', etc.
Applying "[HACK] Disable 'TARGET_RUST_CPU_INFO', 'TARGET_RUST_OS_INFO'"
that I've attached, we're not seeing any change in 'make check-rust'
results, for example.

In my opinion, the current implementation should be backed out from the
main development branch (would also reduce pain in merges from GCC
upstream, as mentioned before), and then be developed (quite possibly
based on the current implementation) individually for all GCC
configurations that we'd like to support (with 'sorry' otherwise), in a
coherent way, instead of trying to guess all possible target options as
done by the current implementation.  And, with all relevant test cases
getting added, of course.  That is, at this time, restrict outselves to
GCC configurations that we're actually supporting and testing.

Have we even figured out which of those target options are actually
mandated for a conforming Rust programming language implementation (that
is, users would potentially rely on these)?

As far as I can tell, 'rustc' defines target options here:
<https://github.com/rust-lang/rust/tree/master/compiler/rustc_target/src/spec>,
and you may use 'rustc --print=cfg' to dump for the current
configuration?

> but getting the normal x86 done correctly will define if
> the other patches are done correctly.

Yes -- but I'm not sure this is it really, in its current WIPy,
un-tested, un-verified form:

> gcc/config/ChangeLog:

>         * gnu.h: add new macro GNU_USER_TARGET_RUST_OS_INFO
>       * dragonfly.h: define TARGET_RUST_OS_INFO
>       * freebsd-spec.h: define FBSD_TARGET_RUST_OS_INFO
>       * freebsd.h: define guard for TARGET_RUST_OS_INFO
>       * fuchsia.h: define TARGET_RUST_OS_INFO
>       * kfreebsd-gnu.h: define GNU_USER_TARGET_RUST_OS_INFO
>       * kopensolaris-gnu.h: define GNU_USER_TARGET_RUST_OS_INFO
>       * linux-android.h: define ANDROID_TARGET_RUST_OS_INFO
>       * linux.h: define GNU_USER_TARGET_RUST_OS_INFO
>       * netbsd.h: define NETBSD_TARGET_RUST_OS_INFO
>       * openbsd.h: define OPENBSD_TARGET_RUST_OS_INFO
>       * phoenix.h: define TARGET_RUST_OS_INFO
>       * sol2.h: define TARGET_RUST_OS_INFO
>       * vxworks.h: define VXWORKS_TARGET_RUST_OS_INFO
>       * vxworksae.h: define VXWORKS_TARGET_RUST_OS_INFO
>
> gcc/config/i386/ChangeLog:
>
>         * crtdll.h: define EXTRA_TARGET_RUST_OS_INFO
>         * cygming.h: define TARGET_RUST_OS_INFO
>         * cygwin.h: define EXTRA_TARGET_RUST_OS_INFO
>         * darwin.h: define TARGET_RUST_OS_INFO
>         * djgpp.h: likewise
>         * gnu-user-common.h: define TARGET_RUST_OS_INFO
>         * i386-protos.h: prototype for ix86_rust_target_cpu_info
>         * i386-rust.cc: new file to generate the rust target host info
>         * i386.h: define TARGET_RUST_CPU_INFO hook
>         * linux-common.h: define hooks for target info
>         * lynx.h: likewise
>         * mingw32.h: likewise
>         * netbsd-elf.h: likewise
>         * netbsd64.h: likewise
>         * nto.h: likewise
>         * openbsdelf.h: likewise
>         * rdos.h: likewise
>         * rtemself.h: likewise
>         * t-i386: add makefilke rule for i386-rust.cc
>         * vxworks.h: define TARGET_RUST_OS_INFO


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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-HACK-Disable-TARGET_RUST_CPU_INFO-TARGET_RUST_OS_INF.patch --]
[-- Type: text/x-diff, Size: 1780 bytes --]

From a5688c5da3c9ffda614f4138e55f46b7078b9e3a Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <thomas@codesourcery.com>
Date: Thu, 28 Jul 2022 12:04:28 +0200
Subject: [PATCH] [HACK] Disable 'TARGET_RUST_CPU_INFO', 'TARGET_RUST_OS_INFO'

---
 gcc/rust/rust-lang.cc            | 2 ++
 gcc/rust/rust-session-manager.cc | 6 ++++++
 2 files changed, 8 insertions(+)

diff --git a/gcc/rust/rust-lang.cc b/gcc/rust/rust-lang.cc
index dd8c608789a..40331f6a431 100644
--- a/gcc/rust/rust-lang.cc
+++ b/gcc/rust/rust-lang.cc
@@ -107,6 +107,8 @@ struct GTY (()) language_function
 void
 rust_add_target_info (const char *key, const char *value)
 {
+  sorry("TODO");
+
   Rust::Session::get_instance ().options.target_data.insert_key_value_pair (
     key, value);
 }
diff --git a/gcc/rust/rust-session-manager.cc b/gcc/rust/rust-session-manager.cc
index 6a2c1b6dd62..d41dc18fa00 100644
--- a/gcc/rust/rust-session-manager.cc
+++ b/gcc/rust/rust-session-manager.cc
@@ -134,6 +134,8 @@ validate_crate_name (const std::string &crate_name, Error &error)
 void
 Session::implicitly_enable_feature (std::string feature_name)
 {
+  sorry("TODO");
+
   // TODO: is this really required since features added would be complete via
   // target spec?
 
@@ -195,6 +197,8 @@ Session::implicitly_enable_feature (std::string feature_name)
 void
 Session::enable_features ()
 {
+  sorry("TODO");
+
   bool has_target_crt_static = false;
 
   rust_debug (
@@ -335,6 +339,7 @@ Session::enable_features ()
 void
 Session::init ()
 {
+#if 0
 #ifndef TARGET_RUST_OS_INFO
 #define TARGET_RUST_OS_INFO()
 #endif
@@ -379,6 +384,7 @@ Session::init ()
 
   // derived values from hook
   options.target_data.init_derived_values ();
+#endif
 
   // setup singleton linemap
   linemap = rust_get_linemap ();
-- 
2.35.1


  parent reply	other threads:[~2022-07-28 10:38 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-27 13:40 Rust frontend patches v1 herron.philip
2022-07-27 13:40 ` [PATCH Rust front-end v1 1/4] Add skeleton Rust front-end folder herron.philip
2022-07-27 13:40 ` [PATCH Rust front-end v1 2/4] Add Rust lang TargetHooks for i386 and x86_64 herron.philip
2022-07-28  9:57   ` Jakub Jelinek
2022-07-28 10:38   ` Thomas Schwinge [this message]
2022-07-28 10:51     ` Philip Herron
2022-07-28 11:08       ` Thomas Schwinge
2022-07-28 11:41         ` Philip Herron
2022-07-27 13:40 ` [PATCH Rust front-end v1 3/4] Add Rust target hooks to ARM herron.philip
2022-07-27 14:13   ` Richard Earnshaw
2022-07-27 16:45 ` Rust frontend patches v1 David Malcolm
2022-07-28  9:39   ` Philip Herron
2022-08-10 18:56     ` Philip Herron
2022-08-10 19:06       ` David Malcolm
2022-08-12 20:45       ` Mike Stump
2022-08-15 14:07       ` Manuel López-Ibáñez
2022-08-15 14:33         ` Martin Liška
2022-08-24 13:48           ` Martin Liška
2022-08-24 13:51             ` Philip Herron
2022-08-24 14:02               ` Martin Liška

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=878roda5f5.fsf@euler.schwinge.homeip.net \
    --to=thomas@codesourcery.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=herron.philip@googlemail.com \
    --cc=philip.herron@embecosm.com \
    --cc=simplytheother@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).