Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reintroduce targetrustm hooks without the maze of macro definitions in target headers #1543

Merged
merged 5 commits into from
Oct 7, 2022

Conversation

ibuclaw
Copy link
Contributor

@ibuclaw ibuclaw commented Sep 24, 2022

  • Adds skeleton support and documentation for targetrustm hooks.
  • Re-adds previously reverted TARGET_RUST_CPU_INFO target hook.
  • Re-adds the previously reverted TARGET_RUST_OS_INFO target hook.
  • Implements TARGET_RUST_CPU_INFO hook for x86 (only a subset of the original i386-rust.cc was re-added)
  • Implements TARGET_RUST_OS_INFO hook for darwin (covers both x86 and powerpc).

There's quite a bit to unpick from what was removed, but I hope this shows how firstly rust-target.h and tm_rust.h should be kept as empty as possible for the default-rust.cc source file that pulls in these headers; secondly all rust macros and declarations be put in rust-specific headers, then pulled into tm_rust.h as-needed. From every other ${target}-rust.cc file, you can include as much as you like in order to pry out the information you need to get at.

I've verified this does work as expected with the -frust-dump-target_options switch, e.g with -mavx2

target_endian: "little"
target_pointer_width: "64"
target_feature: "fxsr"
target_feature: "popcnt"
target_feature: "avx2"
target_feature: "ssse3"
target_feature: "sse4.1"
target_feature: "sse3"
target_feature: "avx"
target_feature: "sse"
target_feature: "xsave"
target_feature: "sse4.2"
target_feature: "sse2"
target_feature: "mmx"
target_arch: "x86_64"

I've also built a few darwin cross compilers with the following results on i686-darwin

target_pointer_width: "32"
target_env: ""
target_vendor: "apple"
target_endian: "little"
target_os: "macos"
target_family: "unix"
target_feature: "sse3"
target_feature: "sse"
target_feature: "sse2"
target_feature: "mmx"
target_arch: "x86"

and on powerpc64-darwin

target_pointer_width: "64"
target_env: ""
target_vendor: "apple"
target_endian: "big"
target_os: "macos"
target_family: "unix"

@ibuclaw ibuclaw force-pushed the ibuclaw/targetrustm branch 3 times, most recently from 87291ba to 7496485 Compare September 26, 2022 10:20
@philberty philberty requested review from philberty, tschwinge and CohenArthur and removed request for tschwinge September 26, 2022 12:04
Copy link
Member

@CohenArthur CohenArthur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks a lot

@ibuclaw
Copy link
Contributor Author

ibuclaw commented Sep 28, 2022

Added darwin-specific target hooks (it's picked up by both x86 and PPC).

Mostly now it's just a matter of adding other OS and CPU support code in the exact same way.

@CohenArthur
Copy link
Member

@ibuclaw do you want to merge this in this state or keep working on it? I see the PR is still marked as draft, but it's in a good spot and I don't mind merging it now

@ibuclaw
Copy link
Contributor Author

ibuclaw commented Sep 28, 2022

@CohenArthur depends on how small you want to keep this. :-)

I can keep incrementally adding targets here until a suitable baseline set of implementations are present, or carry on adding one target, one PR at a time.

Of what's being tested by the GH runners, the only rust target info that's missing would be Linux support.

@ibuclaw
Copy link
Contributor Author

ibuclaw commented Sep 28, 2022

Removed unnecessary include from i386-rust.cc, I guess this is fine for now, and will just add support for other kinds of targets in future PRs, based on what was previously reverted from the original target support code.

@ibuclaw ibuclaw marked this pull request as ready for review September 28, 2022 15:57
@ibuclaw ibuclaw force-pushed the ibuclaw/targetrustm branch 2 times, most recently from 9f62814 to 5688e41 Compare September 29, 2022 08:45
@ibuclaw
Copy link
Contributor Author

ibuclaw commented Sep 29, 2022

Split out the implementation of the target hook from the actual adding of it, so anyone else who's interested can just look at either commit and see clearly what they have to do in order to in add, say, target cpu info for riscv-*-* or os info for *-*-netbsd.

gcc/ChangeLog:

	* Makefile.in (tm_rust_file_list, tm_rust_include_list, TM_RUST_H,
	RUST_TARGET_DEF, RUST_TARGET_H, RUST_TARGET_OBJS): New variables.
	(tm_rust.h, cs-tm_rust.h, default-rust.o,
	rust/rust-target-hooks-def.h, s-rust-target-hooks-def-h): New rules.
	(s-tm-texi): Also check timestamp on rust-target.def.
	(generated_files): Add TM_RUST_H and rust-target-hooks-def.h.
	(build/genhooks.o): Also depend on RUST_TARGET_DEF.
	* config.gcc (tm_rust_file, rust_target_objs, target_has_targetrustm):
	New variables.
	* configure: Regenerate.
	* configure.ac (tm_rust_file_list, tm_rust_include_list,
	rust_target_objs): Add substitutes.
	* doc/tm.texi: Regenerate.
	* doc/tm.texi.in (targetrustm): Document.
	(target_has_targetrustm): Document.
	* genhooks.cc: Include rust/rust-target.def.
	* config/default-rust.cc: New file.

gcc/rust/ChangeLog:

	* rust/rust-target-def.h: New file.
	* rust/rust-target.def: New file.
	* rust/rust-target.h: New file.
gcc/ChangeLog:

	* doc/tm.texi: Regenerate.
	* doc/tm.texi.in: Add @node for Rust language and ABI, and document
	TARGET_RUST_CPU_INFO.

gcc/rust/ChangeLog:

	* rust/rust-lang.cc (rust_add_target_info): Remove sorry.
	* rust/rust-session-manager.cc: Replace include of target.h with
	include of tm.h and rust-target.h.
	(Session::init): Call targetrustm.rust_cpu_info.
	* rust/rust-target.def (rust_cpu_info): New hook.
	* rust/rust-target.h (rust_add_target_info): Declare.
gcc/ChangeLog:

	* doc/tm.texi: Regenerate.
	* doc/tm.texi.in: Document TARGET_RUST_OS_INFO.

gcc/rust/ChangeLog:

	* rust/rust-session-manager.cc (Session::init): Call
	targetrustm.rust_os_info.
	* rust/rust-target.def (rust_os_info): New hook.
There are still quite a lot of the previously reverted i386-rust.cc
missing, so it's only a partial reimplementation.

gcc/ChangeLog:

	* config/i386/t-i386 (i386-rust.o): New rule.
	* config/i386/i386-rust.cc: New file.
	* config/i386/i386-rust.h: New file.
gcc/ChangeLog:

	* config.gcc (*-*-darwin*): Set rust_target_objs and
	target_has_targetrustm.
	* config/t-darwin (darwin-rust.o): New rule.
	* config/darwin-rust.cc: New file.
@philberty philberty self-assigned this Oct 6, 2022
@philberty philberty added this to the Unstable bucket milestone Oct 6, 2022
Copy link
Member

@philberty philberty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!!

Nice work this is really great work! Lets merge this Mark's build farm has a bunch of more esoteric targets that would be really nice to keep green you can see their status in our README near the top.

I think let's merge this. As part of merging gccrs upstream, I think we will likely make this a separate set of patches to make the current review process simpler and then once that's done we can get this reviewed.

@philberty philberty removed this from the Unstable bucket milestone Oct 7, 2022
@CohenArthur
Copy link
Member

bors r+

@bors
Copy link
Contributor

bors bot commented Oct 7, 2022

Build succeeded:

@bors bors bot merged commit bef31ea into Rust-GCC:master Oct 7, 2022
@ibuclaw ibuclaw deleted the ibuclaw/targetrustm branch October 7, 2022 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants