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

rustbuild: is llvm always built with AVR backend? #76470

Closed
matthiaskrgr opened this issue Sep 8, 2020 · 10 comments
Closed

rustbuild: is llvm always built with AVR backend? #76470

matthiaskrgr opened this issue Sep 8, 2020 · 10 comments
Labels
C-bug Category: This is a bug. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Comments

@matthiaskrgr
Copy link
Member

I have this in my config.toml

targets = "X86"
but while x.py runs cmake, I saw

-- Constructing LLVMBuild project information
-- Linker detection: LLD
-- Found Git: /usr/bin/git (found version "2.28.0")
-- Targeting X86
-- Targeting AVR
Traceback (most recent call last):
  File "<string>", line 22, in <module>
IndexError: list index out of range
-- Configuring done
-- Generating done
CMake Warning:
  Manually-specified variables were not used by the project:

    PYTHON_EXECUTABLE
    WITH_POLLY

Making me wonder if there is something that always causes AVR to be activated (by mistake?)

repo @ e82584a

@matthiaskrgr matthiaskrgr added the C-bug Category: This is a bug. label Sep 8, 2020
@mati865
Copy link
Contributor

mati865 commented Sep 8, 2020

Seems like error in #69478

https://github.com/rust-lang/rust/pull/69478/files#diff-9a46b13a265be2ded97369dddf5037f5R147 probably should have been left empty, otherwise this comment is not valid:

# LLVM experimental targets to build support for. These targets are specified in
# the same format as above, but since these targets are experimental, they are
# not built by default

cc @dylanmckay

@matthiaskrgr
Copy link
Member Author

Yeah, I was looking into native.rs and it seems that when no experimental targets are activated, AVR is enabled. I'll push a patch in a few minutes.

@petrochenkov
Copy link
Contributor

This is not an issue with AVR specifically, all experimental targets were always built by default, similarly to how all regular targets are built by default.
So you always had to use

targets = "X86"
experimental-targets = ""

rather than just

targets = "X86"

to disable targets except for x86.

@matthiaskrgr
Copy link
Member Author

That seems counterintuitive to me.
The default according to config.toml.example is indeed #experimental-targets = "AVR" but since it is commented out it makes it look like it's not building that.

@petrochenkov
Copy link
Contributor

petrochenkov commented Sep 8, 2020

I agree that it's counterintuitive, but not building AVR by default will silently disable // needs-llvm-components: avr tests on CI, for example.

Perhaps the better default would be to drop experimental targets only if the targets field was specified explicitly.

@mati865
Copy link
Contributor

mati865 commented Sep 8, 2020

What about disabling experimental-targets by default but explicitly enabling AVR on the fastest CI test job?

@Mark-Simulacrum
Copy link
Member

Hm, I think the comment is probably misleading, but I don't personally see why this is counterintuitive - everything in config.toml.example is commented out, since it's intended to just provide defaults at a point in time. Since many people copy it in its entirety, if they weren't mostly commented we would find it harder to update defaults (and have them actually take effect).

On this case, maybe we can merge AVR into the targets list? Rustbuild could manually separate it out into experimental targets for LLVM, but I feel like from a rustc perspective there's no significant difference between experimental and regular targets.

I am personally a bit hesitant to modify local configurations in a way that diverges from CI to prevent painful debugging as much as possible - do we have some idea how much time cutting LLVM targets saves?

@jyn514 jyn514 added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Sep 8, 2020
@jyn514
Copy link
Member

jyn514 commented Sep 8, 2020

See also #76408

@nikic
Copy link
Contributor

nikic commented Sep 8, 2020

AVR is no longer an experimental target since LLVM 11.

@jyn514
Copy link
Member

jyn514 commented May 20, 2023

I'm going to close this since it seems like it's no longer causing issues.

@jyn514 jyn514 closed this as completed May 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
6 participants