-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
-Calign-jumps=1
support in rustc
#128831
Comments
@rustbot label I-rust-for-linux |
@ojeda Please do not use I- labels for things that are not relevant for determining the actual impact of an issue with respect to the compiler. There is not an I-amazon nor an I-google nor an I-microsoft on the issue tracker. With respect to codegen features, you should link to the relevant LLVM feature support. If Clang does not support it, it is likely LLVM does not. If it does not, then this is an LLVM bug. |
The label was created some days ago by @joshtriplett (thanks!) and if I remember correctly from the meeting, If there is an issue with that (e.g. another label should be used), then please let us know, because we were about to start tagging the issues we have in our lists.
In addition, generally speaking, even if a given backend does not support a particular feature directly, it could be the case that |
So, using an A-label should be fine. Probably it should be a project/working group and have a PG-* label but whatever, that might not be an unauthenticated label set, and I don't much care about that point. I just know that the I- labels are deliberately a somewhat angry-looking red to draw attention to them.
Hm. While hypothetically we can implement something that only "cg_clif" and "cg_gcc" support, it is very difficult to test that something works if our primary codegen backend doesn't implement the necessary prerequisites. Very often, because cg_llvm is more mature, it's the "reference implementation" against which the others are tested. My understanding of how the codegen requirements work here is that it is easy to align function entries with minimal cooperation from the codegen backend, but agonizing to implement that for loops and other jumps without cooperation from the codegen backend. I suspect you don't want this to be "just a hint" like inlining is, and thus easily disregarded, so it seems likely it is raising our requirements for codegen backends. Not a very tall fence, I would think, as "align jumps to N" is often a pretty small ask and something they have to, to some extent, do anyways, but not on the ground. ...Though, reading the GCC documentation, it's not clear to me what the semantics are, actually, for architectures which require an alignment greater than 1 for jumps, as the text claims that "loops are not aligned" (I assume they mean"jumps") if Which would suggest that this is, in fact, also a "just a hint" codegen flag? @ojeda Is the intention that the codegen backend is indeed allowed to simply ignore any alignment values it decides are too low and right-size them to a value it prefers? |
And in what universe does someone want to set loop and jump target alignment differently...? On most architectures they aren't even different things... |
Ah, I see -- I hadn't noticed the color :) For us any prefix is fine really, we normally track things in our lists, but having a tag may be useful for everyone to mark potential things etc.
That makes sense, yeah. I would imagine it is hard to maintain things that only GCC supports without a mature backend. This is not critical for us at the moment, since we mainly support LLVM builds, but eventually we will want to match C on the GCC builds. By then, perhaps the GCC backend is more mature. As for the feature itself -- in the kernel it is only used as I assume it is "just an optimization" for GCC, since it does it only for non-cold ones apparently, so it would be hard to rely on anyway. From what I see in those kernel commits, it sounds like the kernel only uses it as an optimization as well. So even if it was a guarantee by GCC for all non-cold ones, I think a hint would work fine for us. Though if a particular backend guarantees a bit more, it would be nice to provide that via that backend (even if the "surface level option" in As for LLVM, for things that Clang does not implement (yet?) but that may be doable via LLVM, in general, it may be a nice opportunity to provide something better than Clang if they don't expose a particular optimization but Thanks! |
I think the distinction may be around what the manual hints at with "for branch targets where the targets can only be reached by jumping", even if they all end up being implemented with jumps. For instance, playing a bit with both options, I can get one flag to apply to a |
@workingjubilee I'm the one who created the label, and at the time I was not clear on whether it should be an |
It sounds like RfL's use of this is exclusively as a hint for optimization. If we called this For future non-optional functionality that's backend-specific, we don't have a general policy. I think we'd need to consider it on a case-by-case basis, and at a minimum error if asking for it on a backend that doesn't support it. |
Thanks @joshtriplett, I appreciate it. It was fine to get some blame in any case, after all, we discussed the label together :)
Sounds good. For the kernel typical use cases, throwing an error if the backend does not support something would be perfectly fine, since we can tweak flags depending on the compiler, i.e. we don't expect every combination to work (Josh of course knows this -- just adding some context here for others). In fact, for non-optional functionality, I think anything else would be quite surprising/dangerous (at least by default -- perhaps there could be a way to say "it is fine to ignore these particular flags if they do not work for the current backend" in |
Yeah, generally we try to pass on expectations to the backend faithfully and generally the backend upholds those. |
i.e. the equivalent of
-falign-jumps=1
(GCC, Clang does not support it).The text was updated successfully, but these errors were encountered: