-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Introduce the ASM project group #2836
Conversation
The reposity link is currently broken, could someone with the appropriate permissions create |
cc @eternaleye |
This looks great to me. We already discussed this in last week's language team meeting, and this proposal is just to charter the team to work on the problem rather than endorsing any particular solution, so let's go ahead and get consensus and get this rolling! @rfcbot merge |
Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged team members: Concerns:
Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds great!
Is this saying that the resulting design must have rustc treat the asm code as an opaque string, or just that rustc can't be expected to compile the code itself? There is vast middle ground between these two requirements. For instance, the project group might want to define a single comment syntax so higher level tools (like say syntax highlighters) can at least partially interpret the code |
That sounds neat, but also that sounds like something that should wait for the project group to be accepted and then have that discussion of the details there. |
I'm not super familiar with how Rust project groups operate, but my expectation would be that if the charter says something is out of scope, then the project group isn't "allowed" to discuss it. That's why I'm hoping to clarify the group scope before it is accepted/started. (This is of course the wrong place to debate my hypothetical comment syntax proposal, I just wanted to give an example of an issue that I wasn't sure if it was germane or not) |
The scope is intentionally a bit vague here. The intent is that we do not want to have to re-implement a whole assembler ourselves, for every supported architecture. |
I had the exact same reaction while reading this sentence, and hesitated commenting on it. Maybe it'd make sense to just remove that “this is out of scope” line, so that the ambiguity is not there and the project group can just reasonably end up with the conclusion that the solution is not going to be re-implementing a whole assembler? For instance, I seem to remember some pretty convincing arguments saying that all stable asm should actually be at least completely parsed by rustc, if not more, which comes close enough to “re-implement a whole assembler ourselves” to be in the gray zone, as far as I understand. |
I'm guessing this is https://internals.rust-lang.org/t/inline-assembly-syntax/239/5, which was brought up in the recent irlo inline asm thread here: https://internals.rust-lang.org/t/pre-rfc-2-inline-assembly/11310/102 Although I disagree with these arguments, I do think it's reasonable to expect that some future RFC produced by the ASM project group will explicitly say "we didn't go with asm-as-syntax for reasons X, Y, Z", and "because the charter said so" would be a pretty unsatisfying reason. So I'd also be fine with removing that "out of scope" qualifier for now. It's not like there's another Rust project group working on reimplementing assemblers or assembly syntax, and assembly syntax is certainly relevant to the whole discussion of inline asm (as opposed to e.g. how two-phase borrows are technically irrelevant to NLL, and that was worth highlighting on NLL-related issues since it's not obvious). |
Love it :) |
ping @Centril @scottmcm @withoutboats for fcp |
We discussed this some in our lang team meeting last Thursday. @Centril expressed some concerns, though it would be helpful if he could elaborate them, since I don't think I have a great grasp of what they are. One thing that he raised I know is a question of whether we'll have the bandwidth available to tackle this. I personally feel pretty good about this, since I think that
In particular I'd like for the @rust-lang/lang team to get better at being able to "delegate" projects like this. The hope would be that the group can handle figuring out the right path, and keep the team updating both via RFCs and regular updates. As far as priority, I feel that this fits our current project goals in a few ways:
One thing that would be helpful is if we had some folks from @rust-lang/compiler (or @rust-lang/compiler-contributors) who was excited to serve as a dedicated reviewer here. |
🔔 This is now entering its final comment period, as per the review above. 🔔 |
Incidentally, we've just finished the RFC for inline assembly and it is now up at #2850. |
This is probably the wrong thread to be having this discussion, since there's already an RFC for the feature itself, whereas this RFC just sets up a project group. But going point by point:
The plan is to implement 'inline assembly in terms of external assembly' on the rustc side, so that Cranelift doesn't have to do anything at all. I agree this must be done before the new inline asm can be stabilized.
I also hope that the ones proposing the feature will be the ones doing most of the implementation work. That may or may not include me.
You only removed ~1000 lines of code...
But yes, the new one will require more code.
The RFC as currently written does not mention either
I agree, but I believe full or very-near-full parity is achievable.
I disagree that it's niche, and I think you're subjectively underestimating the number of people this is important to.
Strongly disagree that Rust should not "squeeze out every last ounce of performance". There should never be a case where C is faster than Rust (with no plan to fix it). |
I'd like to explicitly +1 this. I'd phrase it this way: If Rust is not at least as fast as any memory-unsafe high-level language, some critical systems will be written in a memory-unsafe language. |
@Centril
I'm not going to claim that this will be trivial, but part of the point of this design was to create something we felt was more manageable, and which insulates us from issues in LLVM's assembly support. The existing inline assembly mechanism is a fairly "raw" export of LLVM's assembly support, along with any fragility it has. The new proposal avoids that fragility.
I have specifically volunteered to handle the language team side of this, as the liaison. Language team time will not be taken up by the day-to-day activities of this project group. That's part of the point of the project group model.
I agree, which is why we're taking the existing feature of inline assembly and finishing it, turning it into something mature that we can feel comfortable stabilizing. I'll respond to your claim that we shouldn't do this at all later in this post; in the absence of that claim, this is absolutely a feature we currently have, continue to need, and should work on the maturity and stabilization path of.
Which is why we're chartering a project group to do the design work, rather than doing it directly in the language team.
I will handle any such inquiries myself; that's the job I'm volunteering for as liaison.
It's clear that you don't see the value in inline assembly. Many others do, and this was extensively discussed on internals. We can talk about the complexity below, but many people find inline assembly essential for their work.
Yes, that implementation is buggy, and regularly generates internal compiler errors. That's why we're building an easier-to-maintain version. Furthermore, as @comex observed, that really isn't much code. And half the files affected only have an import and a single match branch that just says "handle inline assembly too". Far fewer files actually do something specific to inline assembly. That hardly seems "pervasive".
On the contrary, the point of the implementation we want to work towards stabilizing is that it's simpler than the existing implementation, because it doesn't expose all of LLVM's assembly mechanism. The spec should be substantially simpler. (We've never really had a full specification for the existing raw-LLVM-assembly mechanism, and people have had to go read LLVM's documentation for it instead. We do have a full specification for the new mechanism.)
Inline assembly today regularly generates ICEs; I expect the new version to generate less bugs over time, and require less maintenance. I also expect the implementation to be more self-contained than you're suggesting, but in any case, yes, I'm sure there will be at least some ongoing maintenance. And there's a large set of people interested in the feature, some of whom could step up to help with such maintenance. That's part of what we can evaluate in nightly.
This thought seems unfinished. But in any case, you seem to be objecting to the existence of a feature on the basis of potential future features that are not being proposed today, rather than on the basis of what's actually being proposed. (And for that matter, you seem to be objecting to the formation of a project group to work on it, no matter what that group proposes.)
The comment you're linking to very specifically says that they don't think it'll be much work to get to full parity, and explains why. I have no problem committing, up front, to removing (I would also point out that we're doing such a transition at all precisely because so many people use the existing
First, I expect that the more controlled interface we're providing will be much easier for cranelift to support than today's "support everything LLVM does" interface. Second, the current inline assembly proposal can be implemented with zero support from cranelift, albeit with a performance hit. And given that the proposed cranelift backend is intended to optimize for compilation speed rather than performance of compiled code, that seems fine.
This is incorrect, and there have been multiple counterexamples across various internals threads. It's also insulting and dismissive to claim that features you don't personally care about are "niche features". Please stop. This is not a niche feature. Many people have been looking for this for some time, and in some cases this is the feature that keeps them on nightly.
And do, as several responses have already said. Regarding performance, I certainly don't think that we should make tradeoffs in favor of performance over all else in all cases. I absolutely think the tradeoff makes sense in this case. This is not exclusively about performance; it's also about capabilities, and simplicity, and maintainability. There are not intrinsics for every instruction that people need, and there never will be. (And adding them would be more work than this feature.) And external assembly alone is not quite sufficient, for reasons other than just performance. It's much simpler to write an instruction or two via inline assembly, rather than creating a separate file containing a separate named assembly function with a function-call-based interface.
See above regarding this problematic and inaccurate phrasing.
I'd hardly characterize the overall picture as having a "desire for lots of extensions". On the contrary, I expect us to have fewer extensions than the existing The particular message you link to is pretty much exclusively a matter of syntax, in order to be able to provide other things via libs rather than language.
The current proposal very specifically shows how we can implement this without any support from the backend. And while I very much like the Cranelift project, I don't think that it's the call of a backend that isn't part of the Rust project right now to decide whether this feature is important, particularly given that we can implement a compatible-but-slower solution for the Cranelift backend with no changes to Cranelift.
The first of those comments (from 2017) specifically mentions The second of those comments makes the point that the current nightly version of inline assembly is not something we could reasonably stabilize, which I agree with. Which is why we wrote a whole new spec for a simpler version of inline assembly that we believe we can reasonably stabilize.
This has already been answered by other comments. In short, this has nothing to do with monomorphization, post-monomorphization, or generics. This is simply an inherent as
So, to be clear, by blocking the project group RFC (as opposed to any specific proposal), you are saying you wish to block people from working on a proposal to improve inline assembly from its curent state and stabilize inline assembly. You're not raising specific concerns; you're blanket-rejecting any possible proposal, to improve something that's already part of nightly Rust. (Which means that the current nightly-only less-stable implementation will be staying around longer. Blocking this will not make the existing implementation go away, it will keep that implementation longer.) I believe that by doing so, you are not only blocking productive work by others that does not ask anything of you, you are also blocking progress towards goals you are otherwise stating that you favor (maturity, stability). And I further believe that your post demonstrates a lack of regard or empathy for points of view other than your own. EDIT: In the interests of clarification, I'd like to make it clear that this is a statement of the feelings that the previous post brought up. It feels like the previous posts were pushing back on the existence of inline assembly at all. It feels like we've been over and over the reasons that inline assembly are important, and it feels like these arguments are not being heard. I wanted to make that frustration clear, and to re-present summaries of information from previous responses and discussions in one place. |
I wanted to refrain from speaking on this thread or on the associated RFC, for reasons that should become clear soon, but here it is. I come forward speaking as someone who did embedded work as a dayjob, and will likely in the near future, despite the fact it's not my present right now. I really, really want the code that currently uses inline assembly to be possible to compile on stable. But I agree with @Centril's concern that we should not do this now. The reason why I have been refraining from speaking was that I believed that this would consume time that would have been better used at other things. In particular, finishing off all the in-flight features that are currently waiting (off the top of my head, const generics and async/await, at least). I was also thinking that the current RFC was not too bad, and did not deserve more time spent on it than it was having. However, I do believe that having this project group now is trying to rush inline assembly to stabilization, and that the resources allocated to it (including your time off lang-team spent in it, the development work done by people in the project group, etc.), that would be better allocated finishing these other features first, and then when they are finished, switched back to finding a proper path to inline assembly. For instance, having one intrinsic per instruction, despite being more work, would probably make more sense to me -- at least, off the top of my head, I think it would probably be able to handle all the inline assembly I've been doing, except for one particular snippet (a stack switching one, that probably would have been better done as external assembly anyway, as AFAIR it was in a I am not trying to argue in favor or against that specific RFC. The current one is good enough, and if we want to get something done now, with the resources currently available, it is probably the best bet. However, I do agree with @Centril that the project group should not be chartered for the time being, until the resources available to it allow basically any solution to be implemented, and in particular so that the language does not unnecessarily definitely end up tied to the current implementation details of the compiler, that happens to be gas-compatible. And I believe that, despite the fact that I've been longing for stable inline assembly for a few years -- the currently available resources just appear, to me, insufficient to reasonably encompass the full design space available. EDIT: I will probably not submit further messages to these threads, as that would go against my “let's try to focus on other work” objective. I hope this message's content is clear, and will let other people decide of what to do with the information therein. |
I'm not sure where that impression would come from. I expect the new inline assembly to spend a long time in nightly for experimentation before we consider stabilizing it. We're trying to implement a version of inline assembly that we have a hope of stabilizing in the future, but we have no interest in rushing that process.
Part of the point of this effort is to reduce the surface area of the feature to make it more maintainable, which this would not do. Also, many kinds of assembly cannot work via intrinsics, as they require control not just over the instructions used but the exact block of instructions emitted.
My time in the language team, and the time of the other people in the project group, isn't a generic "resource" that can be "better allocated". We're spending time on this because we need inline assembly support specifically. We'd love to finish other features, too, but that isn't mutually exclusive. |
Throwing out concerns and then immediately saying "also I won't come back and discuss this further" is unfortunate to say the least. |
@Lokathor I simply think there's nothing more I could say, should questions arise on my concerns I'll answer them, but trying to argue further would mean spend even more time from the people reading me, and that goes against the entire objective my post was striving towards ;) @joshtriplett Sorry if the choice of the word “rush” was bad, what I meant was “later could be a better moment to do it, with fewer inflight projects, and it is not particularly more urgent now than a year ago” (this is not an argument towards never doing it and always pushing it back to later, either, I think once either async/await or const generics get completed then it could be a great project, but three simultaneous huge projects sounds like too many to me) |
I think that the end of Josh's most recent reply is the most important fact to hold in mind when talking about any of this: Just because it's decided "We're not working on A, we're working on B", that doesn't mean people who would have worked on A will have the skills or interest level to work on B instead. Volunteer time is not fungible like that. Specifically you mentioned async/await twice now, and while I'm sure it's very nice to some portion of Rust users, it is also totally useless to a different portion of Rust users. Telling the embedded development people of Rust "We don't want to even let you discuss how you could improve your usage of Rust, because the web app developers need to get more attention instead" is not a very good argument. It really doesn't make sense at all. The people working on one area aren't the same people working in the other. They should be totally separate efforts, each allowed to progress at their own pace according to how much interest and support they're able to have people put in. Similarly for const generics. Very cool language feature, and people are excited for it, but the tracking issue which was opened 2 years ago still has only 1 of 15 boxes checked (including "design" being unchecked), and there's 3 pages of related issues in the issue tracker. It's simply unreasonable to imagine that it would all suddenly get sorted out in, say, 6 months or less. So saying that discussion of inline assembly is going to be blocked until const generics is entirely sorted out doesn't make any sense. |
There are people with the requisite knowledge and skills, eager to invest their time into this feature today. That may not be the case in six months time, or whenever inline assembly reaches the top of everyone else's priority list. If there's no question that Rust should support inline assembly at some point, that the people in the proposed group are qualified, and (most importantly) that inline assembly is entirely orthogonal to/won't conflict with any other features in the works, then the language team should be able to accommodate that. The Rust teams exist to create some kind of order out of the chaos that is open source, and prioritising features is a tool to do that, but if taken to the extreme then it just becomes the Rust teams telling people what they can or cannot work on, and that's a sure-fire way to kill people's enthusiasm for open source because it just becomes an unpaid job. |
The way I see it, the current direction is the least bad option that still meets the needs of embedded/low level developers. It would be really nice if adding a bunch of intrinsics was enough for everyone. It would be really nice if linking to external assembly was enough. Or if there were the resources to design and write our own assember. Or if we had the expertise to write a custom DSL. Or... But the reality is that none of those fully solve the issue within the constrains we have. Inline assembly may be inferior to each of them in some dimension, but only it is viable solution. |
I think this conversation about resourcing -- and how to think about it -- is very important. I've been advocating for some time now that the Rust project needs to think more about followthrough and resourcing. This was the point of my unbounded queues blog post, for example. Still, I strongly agree with @Diggsey that we have to balance this sort of planning with a respect for serendipity and momentum (and indeed the post talked about this, too). I also think we need to think about the balance of skills. I don't think that folks pushing on const generics will necessarily slow down progress on const generics, for example. In my view, one of the main purposes for the Rust organization is to help people help themselves. In some ways, we're really great at that, with RFCs, mentoring, and the like. But for larger efforts, it's harder. There are also a lot of challenging problems that have "half-finished" designs that we just can't seem to get over the finish line. I would put inline assembly into this bucket, but also things like custom test frameworks, const generics, custom allocators, specialization, etc. I frequently get asked by people "how can I make feature X happen?" and the bottom line is that I usually don't have a good answer for them. Indeed, the whole project group concept is basically a response to these two concerns: balancing the dangers of unbounded queues with the desire to help people help themselves. (And adding in the idea of creating a structure that can help with mentoring and getting people involved in the Rust project.) My hope is that the answer to someone who says "I want to see feature X happen", from now on, will be:
It's absolutely true though that we can't do all the things at once. This is part of the reason to ensure that we have specific team liaisons and sponsors: so long as nobody is liaison for more than 1 or 2 projects at a time, this should naturally tie the number of active projects to the bandwidth of the team. I also think it's the right model for our teams: team members should be serving as "multipliers", helping to enable people. Anyway, enough waxing philosophical. I will just close by noting that -- of course -- we also have to be careful about what we do. Just because people are excited to push some design doesn't make it a good idea. But I think that in the case of inline assembly (and many of the examples I listed above) there have already been a number of excellent comments about why it's important, so I don't have that much to add there. |
I was thinking about what it will take to move this conversation towards consensus. I thought it would be useful to start with what are the key things under contention.
Does this sound about right? Looking at the second question in particular, I would say that we might consider several kinds of work. Here is my first stab at a list:
|
Another necessary task is to design and implement the inline-assembly-as-external-assembly shim. |
Yes, I meant for that to be "compatibility with other backends" -- though it's worth asking ourselves how important we think this is. In any case, I updated the list of work items. |
One thing I am wondering about -- are there folks on @rust-lang/compiler or @rust-lang/compiler-contributors who are interested in (and feel they would have time for) serving as the "main reviewer" for this work? (I imagine some aspects of it, like integration into the borrow checker, might want double-checking by @matthewjasper and/or myself, but I think the majority of this is going to be about checking that the constraints are met, that things are parsed etc correctly, and that we translate to LLVM and/or the backend correctly) As @Centril said, there will always be maintenance work to consider, and I definitely think we'd want to be growing multiple people with knowledge of how the code works. (Ideally, this would also happen as part of the project group.) I thought @nagisa might be into this, but I'm not sure how much time they've got. =) Edit: Oh, I see I asked this earlier. |
I always appreciate some good code to read and should be available to review inline assembly PRs too. |
Is there much point on continuing to block the project group RFC? At this point, we've already published the RFC (#2850) and incorporated feedback from the community into it. We even have an implementation of the RFC (rust-lang/rust#69171) which is 90% complete. |
OK, this has obviously been stalled for some time. I feel we need to push this towards resolution. As of this writing, everyone in the @rust-lang/lang team has checked their box apart from @Centril, who has raised blocking objections in this comment. The comment is quite detailed and worth a read, but I believe the core points are as follows (@Centril, feel free to let me know if you feel I overlooked or de-emphasized something):
Since those concerns were posted, there have been a number of replies, as well as further developments. Most notably the project group has been active, the My feeling is that most of the above points have been addressed. I'm going to briefly summarize:
In the interests of driving this discussion to a conclusion, I would like to ask a few things:
Note that my goal here is not to convince you necessarily (though that'd be nice!), but I want to make sure we understand each other's takes. |
Just a quick update: the PR implementing the new |
It seems that this discussion has deadlocked. @Centril has valid concerns which should definitely be resolved prior to stabilizing
The new |
I've discussed this over with the @rust-lang/lang team and our consensus is that we should move forward with this project group. The team feels that the concerns @Centril raised have been sufficiently addressed by the responses posted above, as well as the events that have occurred in the meantime, such as the work on RFC #2850. Moreover, even without this RFC being accepted, the project group was active regardless, and has indeed nearly completed its work, so it's evident that this particular RFC should be accepted. Given that @Centril has stepped back from the language design team, I'm going to go ahead and resolve the concerns they left on this issue on their behalf (let's see if this works...): @rfcbot resolve i-dont-think-we-should-do-this |
(The resolutions aren't acknowledged in #2836 (comment)) |
Yeah, I thought that would work but I guess it didn't. I've notified @anp but that's ok. I'm just going to "cancel" the FCP and handle it manually, since the requisite 10 day period has long since expired. |
@rfcbot fcp cancel |
@nikomatsakis proposal cancelled. |
Huzzah! The @rust-lang/lang team has decided to accept this RFC. |
Rendered