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.

Sign up for GitHub

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

Use fixed architecture names in toolchain wrapper #43

Merged
merged 1 commit into from
Mar 10, 2025
Merged

Use fixed architecture names in toolchain wrapper #43

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions toolchains/lowrisc_rv32imcb/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ HOST_ARCHS = [
[filegroup(
name = "compiler_components" + host_arch,
srcs = [
"//toolchains/lowrisc_rv32imcb/wrappers:all",
"//toolchains/lowrisc_rv32imcb/wrappers_{}:all".format(host_arch),
"@lowrisc_rv32imcb_{}_files//:all".format(host_arch),
],
) for host_arch in HOST_ARCHS]
Expand All @@ -43,15 +43,15 @@ HOST_ARCHS = [
},
substitutions = device.substitutions,
tools = {
"ar": "wrappers/ar",
"cpp": "wrappers/cpp",
"gcc": "wrappers/clang",
"gcov": "wrappers/gcov",
"ld": "wrappers/ld",
"nm": "wrappers/nm",
"objcopy": "wrappers/objcopy",
"objdump": "wrappers/objdump",
"strip": "wrappers/strip",
"ar": "wrappers_{}/ar".format(host_arch),
"cpp": "wrappers_{}/cpp".format(host_arch),
"gcc": "wrappers_{}/clang".format(host_arch),
"gcov": "wrappers_{}/gcov".format(host_arch),
"ld": "wrappers_{}/ld".format(host_arch),
"nm": "wrappers_{}/nm".format(host_arch),
"objcopy": "wrappers_{}/objcopy".format(host_arch),
"objdump": "wrappers_{}/objdump".format(host_arch),
"strip": "wrappers_{}/strip",
},
) for device in DEVICES]
for host_arch in HOST_ARCHS
Expand Down
0 toolchains/lowrisc_rv32imcb/wrappers/BUILD → ...s/lowrisc_rv32imcb/wrappers_aarch64/BUILD
View file
Open in desktop
File renamed without changes.
0 toolchains/lowrisc_rv32imcb/wrappers/ar → ...ains/lowrisc_rv32imcb/wrappers_aarch64/ar
View file
Open in desktop
File renamed without changes.
0 toolchains/lowrisc_rv32imcb/wrappers/clang → ...s/lowrisc_rv32imcb/wrappers_aarch64/clang
View file
Open in desktop
File renamed without changes.
0 toolchains/lowrisc_rv32imcb/wrappers/cpp → ...ins/lowrisc_rv32imcb/wrappers_aarch64/cpp
View file
Open in desktop
File renamed without changes.
2 changes: 1 addition & 1 deletion ...hains/lowrisc_rv32imcb/wrappers/driver.sh → ...wrisc_rv32imcb/wrappers_aarch64/driver.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
# SPDX-License-Identifier: Apache-2.0

PROG=${0##*/}
TOOLCHAIN="lowrisc_rv32imcb_$(uname -m)_files"
TOOLCHAIN="lowrisc_rv32imcb_aarch64_files"
Copy link
Collaborator

Choose a reason for hiding this comment

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

It'd be nice if there was a way for bazel to pass this env var in as part of the tool configuration. I did a bit of looking, but I didn't see anything obvious.

I'm also a bit surprised that uname is not available.

Copy link
Contributor Author

@troibe troibe Mar 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Yep that surprised me too. There were only a few specific sandbox environments where it failed to find uname.

  = note: external/crt/toolchains/lowrisc_rv32imcb/wrappers/clang: line 7: uname: command not found
          external/crt/toolchains/lowrisc_rv32imcb/wrappers/clang: line 13: /home/runner/.cache/bazel/_bazel_runner/009b5905a925a435539a99f3bc28ac3e/sandbox/processwrapper-sandbox/2923/execroot/lowrisc_opentitan/external/lowrisc_rv32imcb__files/bin/riscv32-unknown-elf-clang: No such file or directory

Copy link
Collaborator

Choose a reason for hiding this comment

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

It is possible to pass environment variables to tools as part of the toolchain configuration but unfortunately it is a rather non-trivial change because it requires to use env_set and env_entry which we have no reified in the crt (I do have a patch for that but it's quite useless at this point since we do not use the crt for upstream). If you really do need it I can make a PR for that.

Copy link

Choose a reason for hiding this comment

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

I wonder if the issue is that PATH is hidden by Bazel so non-builtin commands will not work at all?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just wanted to quickly follow up on this. @jwnrt @cfrantz
I have to admit I got a bit confused about the feedback.
Is there something straightforward to improve here or is this ok to merge & release?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think let's merge and release this and we can investigate the uname issue later if it becomes an issue

PREFIX="riscv32-unknown-elf"

ARGS=()
Expand Down
0 toolchains/lowrisc_rv32imcb/wrappers/gcc → ...ins/lowrisc_rv32imcb/wrappers_aarch64/gcc
View file
Open in desktop
File renamed without changes.
0 toolchains/lowrisc_rv32imcb/wrappers/gcov → ...ns/lowrisc_rv32imcb/wrappers_aarch64/gcov
View file
Open in desktop
File renamed without changes.
0 toolchains/lowrisc_rv32imcb/wrappers/ld → ...ains/lowrisc_rv32imcb/wrappers_aarch64/ld
View file
Open in desktop
File renamed without changes.
0 toolchains/lowrisc_rv32imcb/wrappers/nm → ...ains/lowrisc_rv32imcb/wrappers_aarch64/nm
View file
Open in desktop
File renamed without changes.
0 toolchains/lowrisc_rv32imcb/wrappers/objcopy → ...lowrisc_rv32imcb/wrappers_aarch64/objcopy
View file
Open in desktop
File renamed without changes.
0 toolchains/lowrisc_rv32imcb/wrappers/objdump → ...lowrisc_rv32imcb/wrappers_aarch64/objdump
View file
Open in desktop
File renamed without changes.
0 toolchains/lowrisc_rv32imcb/wrappers/strip → ...s/lowrisc_rv32imcb/wrappers_aarch64/strip
View file
Open in desktop
File renamed without changes.
19 changes: 19 additions & 0 deletions toolchains/lowrisc_rv32imcb/wrappers_x86_64/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package(default_visibility = ["//visibility:public"])

exports_files(glob(["*"]))

filegroup(
name = "all",
srcs = [
"ar",
"clang",
"cpp",
"gcc",
"gcov",
"ld",
"nm",
"objcopy",
"objdump",
"strip",
],
)
1 change: 1 addition & 0 deletions toolchains/lowrisc_rv32imcb/wrappers_x86_64/ar
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
driver.sh
1 change: 1 addition & 0 deletions toolchains/lowrisc_rv32imcb/wrappers_x86_64/clang
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
driver.sh
1 change: 1 addition & 0 deletions toolchains/lowrisc_rv32imcb/wrappers_x86_64/cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
driver.sh
16 changes: 16 additions & 0 deletions toolchains/lowrisc_rv32imcb/wrappers_x86_64/driver.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
#!/bin/bash --norc
# Copyright lowRISC contributors.
# Licensed under the Apache License, Version 2.0, see LICENSE for details.
# SPDX-License-Identifier: Apache-2.0

PROG=${0##*/}
TOOLCHAIN="lowrisc_rv32imcb_x86_64_files"
PREFIX="riscv32-unknown-elf"

ARGS=()
POSTARGS=()

exec "external/${TOOLCHAIN}/bin/${PREFIX}-${PROG}" \
"${ARGS[@]}" \
"$@"\
"${POSTARGS[@]}"
1 change: 1 addition & 0 deletions toolchains/lowrisc_rv32imcb/wrappers_x86_64/gcc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
driver.sh
1 change: 1 addition & 0 deletions toolchains/lowrisc_rv32imcb/wrappers_x86_64/gcov
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
driver.sh
1 change: 1 addition & 0 deletions toolchains/lowrisc_rv32imcb/wrappers_x86_64/ld
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
driver.sh
1 change: 1 addition & 0 deletions toolchains/lowrisc_rv32imcb/wrappers_x86_64/nm
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
driver.sh
1 change: 1 addition & 0 deletions toolchains/lowrisc_rv32imcb/wrappers_x86_64/objcopy
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
driver.sh
1 change: 1 addition & 0 deletions toolchains/lowrisc_rv32imcb/wrappers_x86_64/objdump
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
driver.sh
1 change: 1 addition & 0 deletions toolchains/lowrisc_rv32imcb/wrappers_x86_64/strip
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
driver.sh
Loading