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

Support armv7l-hf #13738

Merged
merged 3 commits into from
May 28, 2014
Merged

Support armv7l-hf #13738

merged 3 commits into from
May 28, 2014

Conversation

buttslol
Copy link

This was required to get ./configure to work on my armv7 test machine.

I haven't found anything sane to feature gate hf on that's pokable from the context of the configure script.

It also seems that gcc doesn't work on armv7 by default (rust wants to pass it -m32 which isn't supported), would it be preferential to make the default --enable-clang on arm, or remove the -m32 flag on that platform?

@alexcrichton
Copy link
Member

This seems odd to single out only armv7l in the makefiles which seems like an obscure architecture... I hope we can avoid having a huge amount of configuration for each various architecture rust can be compiled to.

@buttslol
Copy link
Author

Yeah, I felt exactly the same way.

Thought on how to generalise it are very welcome- I have only a very minimal ARM test environment.

I can apply all of these changes to arm, my hesitation stems from the assumption that it's presently working on for other arm platforms. Should I just apply it to all arm and see what breaks/who yells at me?

@richo
Copy link
Contributor

richo commented May 7, 2014

Having given this a bit of thought, my two main goals are:

  • Building for arm should be easier than it currently is
  • It should do something correct by default, but be easy and fairly magic to make do something more plausible.

To this end, I want to update the configure script to default arm builds to a pretty sane set of defaults that will probably work everywhere and add some flags to specialize, as well as an auto flag, not actually called auto to make a best effort guess at doing something reasonable.

@chris-morgan
Copy link
Member

For reference, I have gone through three secondary OSes on my Android + Something Else device:

  • Ubuntu, armel: something (arm, I think) and gnueabi.
  • Ubuntu, armhf: the same something and gnueabihf.
  • Debian, armhf: armv7l and gnu (i.e. what @buttslol is reporting).

@richo
Copy link
Contributor

richo commented May 12, 2014

I guess another option is to leave the defaults as is, and add a 'sniff arm' option that shells out to another (bundled) tool that does it's best to work out a reasonable set of options for the thing that you're on.

@alexcrichton
Copy link
Member

I'm worried about this patch singling out one specific ARM architecture, when the architecture itself has little to do with the fix. It seems the real issue here is that the compiler doesn't accept an -m32 argument.

@chris-morgan
Copy link
Member

@alexcrichton Could we land this as is and then gather more data later? Rather than aiming for perfection immediately, I believe that landing this is reasonable; we’ll find out how the situation lies and can make further changes later. It’s not like it’s a big change.

@zwarich
Copy link

zwarich commented May 23, 2014

Why don't you simply remove -m32 from all ARM builds?

@richo
Copy link
Contributor

richo commented May 25, 2014

I'm open to nuking -m32 from all ARM builds if noone objects (we can deal with aarch64 later when someone needs it?)

@zwarich
Copy link

zwarich commented May 25, 2014

AArch64 will hopefully be specified by a different -arch or -triple, not -m32. I know it's that way with Clang, but I'm not sure about GCC.

@richo
Copy link
Contributor

richo commented May 25, 2014

Cool. I'm going to update this PR.

@luqmana
Copy link
Member

luqmana commented May 25, 2014

First off, thanks! I've been meaning to get building on arm work (in a non-adhoc manner).

I think it'd be better to just remove all the -m32 flags sprawled around and instead just have it one place. So something like:

case "$CFG_C_COMPILER" in
    ("ccache clang")
    ...
    ;;
    ("gcc")
    ...
    ;;
    # etc
esac

case "$CFG_CPUTYPE" in
    (x86*)
    LLVM_CXX_32="$LLVM_CXX_32 -m32"
    LLVM_CC_32="$LLVM_CC_32 -m32"
    ;;
esac

That way we wouldn't have to have all those comparisons against armv7l since there are definitely other platforms that'll have the same issue.

Also, for the libcpp stuff, I think it'd be better if there was a flag like ENABLE_LLVM_LIBCPP and then in the armv7l case where we're setting CFG_CPUTYPE we can set that flag to 0. That way if any other platform needs to enable/disable they can easily do and we avoid having to single out just armv7l.

It'd also probably be fine to just squash those commits into one. With all that, r=me.

@richo
Copy link
Contributor

richo commented May 28, 2014

I just updated this PR.

It's going to take a long time to test it properly (my arm machine is a chromebook.. rust builds take their time) and unfortunately this doesn't get it to the point where a simple:

./configure
make

does the right thing, but it gets it much, much closer.

bors added a commit that referenced this pull request May 28, 2014
This was required to get ./configure to work on my armv7 test machine.

I haven't found anything sane to feature gate `hf` on that's pokable from the context of the configure script.

It also seems that gcc doesn't work on armv7 by default (rust wants to pass it `-m32` which isn't supported), would it be preferential to make the default `--enable-clang` on arm, or remove the `-m32` flag on that platform?
@bors bors merged commit b4e69d4 into rust-lang:master May 28, 2014
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 13, 2023
fix: Increase token limit of proc macro

Fixes rust-lang#13738

[The swc project](https://swc.rs) has lots of type definitions, and the current limit is too low for the ECMAScript/CSS visitors.

[rustdoc](https://rustdoc.swc.rs/swc_ecma_visit)

---

With this fix, the rust-analyzer shows trait-related actions.

<img width="1840" alt="image" src="https://user-images.githubusercontent.com/29931815/206839269-d7a04589-7dba-449b-ad0b-1f69d52bd039.png">
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants