-
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
List of ARM assembly directives that the Clang integrated-as cannot handle. #18573
Comments
assigned to @rengolin |
|
The .end directive does not belong on this list. It is not ARM specific. If you like, you can create a new bug and assign it to me. Ive already sent out a review to address that directive. |
Im not sure if we even want to support .extend, .ldouble, or .packed. These are for writing out 12-byte doubles and floats and are not compatible with current CPUs or ABIs AIUI, or am I missing something with regards to those directives? |
Self-contained assembly file with various things clang can't handle
gcc accepts the whole file. clang rejects almost half of it. |
SVN r197657 adds support for .inst. |
197787 adds the .pool directive alias. |
In 197547, Saleem added .end |
Do not belong on this bug. These are generic IAS issues, not particular to ARM.
Sent out a review for this. |
Hi Saleen, Not all directives / problems listed in this bug will be ARM-specific, but they will all impede the ARM IAS from working properly, thus will be in our list for fixing before 3.5, so that we can have a fully-functional assembler, with just some very specific cases falling back to GAS. The reports here are all by people working on the ARM IAS, so it should be ok to keep them here... |
r197913 takes care of the default immediate for bkpt. |
While waiting on reviews, Ive pushed some of my changes into a temporary repository on github if anyone else wants to play with this. https://github.com/compnerd/llvm/commits/development .even, .inst, .movsp, .pool, .end, .rept, and bkpt default immediates are taken care of. |
Okay, I have a patch for .personalityindex, but it really depends on a refactoring of the unwind context. But, I think the result is much nicer and provides much nicer diagnostics. Ive pushed it to the repository on github, Ill see about posting patches for review prior to pushing when people are back from the holidays. |
r198031 adds .even in SVN |
Hi Saleem, About .personalityindex, I'd talk to Anton Korobeynikov and Logan Chien first, as they was working on the EHABI changes for a while. |
Just to keep track of items, I didnt realise that Logan had a change for eabi_attributes, and I independently addressed that (Ive sent an email to Renato and Logan to discuss how to proceed with regards to that). The eabi_attribute change is in the git repository as well. |
SVN r198097 adds .rep support. |
I have a change to address the immediate operands requiring label arithmetic that Ill be sending out soon for review. |
Ill send out a review for the aliases for vpush/vpop. As far as the attached sample, there is only the ldrd/strd mnemonics with the implicit Rt2 and the named parameters in the macro assembler that seems to be remaining. |
SVN r198172 adds the FLDM and FSTM aliases. |
Just sent out a review for the ldrd/strd GNU extension for the single register form. Taking into account pending changes, the only thing in the sample that Simon has attached to this report which does not get handled correctly is the macro parameters. |
SVN r198662 improves the eabi_attribute handling. |
SVN r198735 addresses the immediate expression handling. |
Just to make sure we're up-to-date on the progress, what's still missing: Not started:
In review:
EHABI:
|
I've started looking at this.
Shouldnt this be part of EHABI? I also have a WIP patch for this.
SVN r197787
I started looking at this, but never finished it as I ran into some trouble with proper behaviour for undefined symbols.
I have a partially correct patch for this, that I will try to finish up once I get the llvm-readobj stuff fixed.
|
Hi Saleem, Great! Whenever you start working on something, add a comment to this bug, so other know they shouldn't work on it, too. Work in progress:
In review:
TODO, by type: Architecture definition:
Thumb function alias:
SIMD aliases:
Floating point support:
Exception Handling:
Thread support:
|
.unreq is already supported by the parser. Work in progress:
In review:
TODO, by type: Architecture definition:
SIMD aliases:
Floating point support:
Exception Handling:
Thread support:
|
Yes, I only added it to make it sure we test it with .dn/.qn, too. |
The macro handling is an entire set of problems on its own (there is more there than just named parameters that does not work). The current state of affairs: Work in progress:
In review:
TODO, by type: Architecture definition:
SIMD aliases:
Floating point support:
Thread support:
|
Work in progress:
In review:
TODO, by type: Architecture definition:
SIMD aliases:
Floating point support:
Thread support:
|
I have .tlsdescseq completed and will be submitting that soon. .object_arch requires another change in review (for tests). .movsp just needs to be submitted as well. Work in progress:
TODO, by type: Macro Assembler:
Architecture definition:
SIMD aliases:
Floating point support:
|
.arch_extension cannot be fully implemented correctly in LLVM at this time. I will be sending out a review to cover as much of it as possible soon. Work in progress:
TODO, by type: Macro Assembler:
SIMD aliases:
Floating point support:
|
I'm having a look at .dn/.qn aliases. |
Awesome! I briefly looked at them. They are fairly tricky as you can end up changing mnemonics based upon the aliasing. Laned aliases are also slightly problematic with the current structure in a couple of different places. Storage of the data type indicators cause some problems as well. |
Yes, that's what I'm looking into right now. It should be fairly localized, since this is local to the assembly file, so we should just need to change the parser to look for the register's added properties (type) when parsing SIMD instructions.
This is the next step, but I think some changes in the table-gen will be needed, or maybe we can cheat and directly transform the lane (dN[1]) into the corresponding aliasing sub-register (s2N) and then tweak the printer to display back the laned double/quad.
I haven't looked at this yet. Do you have some examples on the possible syntax? GNU assembler's help pages have a simple example and the manual has a big TODO in the ARM-specific part. |
Sorry about the delay in the response, I've been a bit busy with work. Yeah, I have some examples from when I was playing around with this, that I will upload. In general, this is the syntax: [name] [.dn|.qn] [register name|integer constant expression]{.data type}{[constant integer expression]} The register names can be specified as an index which can be an arbitrary expression, BUT MUST NOT BE PREFIXED (that is #constant is error). The data type is optional, though, at least the last operand or the instruction must be typed to permit type inference. The type may be part of the alias or may be tagged onto the operand. The other operands may be tagged, and if they are, they must match. However, the mnemonic and the operands may not both be tagged. Lane definition is an optional constant integer expression. You can recursively define aliases (that is a subsequent alias can alias another alias). |
alias-diagnostics. |
starting patch, covering the basic cases Missing features:
I'll have to stop this for a while, if anyone want to continue, please do so. Otherwise, I'll pick this up later. Saleem assembly and diagnostics files will give you a more complete idea on what to implement. Also, I'm dubious to what benefit will this case bring to the assembler at this cost. We'll need changes to ARMOperand, parseOperand(), parseInstruction() and maybe even creating a new class of instructions that could accept special aliased registers. I personally believe that .req can do most (if not all) .dn/.qn can without the added complexity. The only benefit would be to compile legacy code, and that was never a priority for the integrated assembler. I'm sceptical. |
Speaking to Gabor, it seems there was no requirement to have .dn/.qn implemented (no source code with it), so I think we should just forget about it and focus on other, more important ones, like macros, which is currently failing the kernel build. |
kernel build log with -k and integrated assembler |
Modern code does use it, and understandably so (skia is one example). I think its fine to priortise other things first; Ive been doing that constantly. I'll look at the macro handling, and come back to the remaining directives later. |
I appreciate that! And I appreciate your efforts to make the IAS better, I'm not complaining or arguing against it, I really welcome it! But there is a design goal that we can't overlooked... There are two reasons to add GNU extensions of pre-UAL support to the integrated assembler:
In the specific case of .dn/.qn aliases, the implementation will not fit around already implemented cases (I thought it could just wrap .req, it can't). It'll add a lot of complexity to the already convoluted assembler code, so it would have to be so widespread that it'd be impossible to not implement it. I don't think that's the case.
I just checked out their code and couldn't find any occurrence in skia. I'm not saying this is not important, just that I can't see the importance of it right now. I can also see the mess it'll create on the current code, which would need to be refactored extensively. As you said, let's leave it for later. My point is just that we shouldn't need its support to call the integrated assembler complete and production quality, since that's just a GNU extension. Once you finish the macro support, we shall see what's left in the kernel, and I'm inclined to call it a day if the kernel compiles. We can (and should) create new bugs for each specific directive that we don't support and discussions about their validities should go in there. |
Okay, for the particular error that you demonstrated, I have pushed a fix for in SVN r201474. I am trying to figure out how to build the kernel locally with clang. The assembly being generated via macros looks invalid to me (of course I may be missing something). For some reason, I am seeing this: -> NR_PAGEFLAGS #22 __NR_PAGEFLAGS This is generated as a result of the DEFINE macro defined in kbuild.h. If you have any suggestions as to what I may doing incorrectly, it would be appreciated. |
Ugh, this is stupidly silly. I think I may have just been scarred for life. The Linux kernel does something patently stupid. They rely on multiple things to work in a very particular manner to generate constants. First: asm volatile will emit crap as instructed, expanding the parameters as necessary. The garbage portion of the second point is IMPORTANT. By doing this, they can rely on the frontend translating constant expressions into values that will be emitted as raw output. This output is then saved off and post-processed by a sed script to generate future input for the compiler. As an example: #define DEFINE(sym, val) asm volatile("\n->" #sym " %0 " #val : : "i" (val)) DEFINE(NR_PAGEFLAGS, __NR_PAGEFLAGS) ->NR_PAGEFLAGS #22 __NR_PAGEFLAGS |
Saleem, The LLVMLinux project has already solved most of the build problems with the kernel and they have their own repositories with their patches needed to build the kernel with Clang: http://llvm.linuxfoundation.org/index.php/Main_Page You should check-out their repository, and build with their own scripts. This will make sure we're tracking the same problems, so that they can coordinate the patches upstream (to the kernel). |
SVN r201499 improves macro initialisation for keyword arguments. At this point the entire file that Simon posted can be properly handled by the IAS! |
Yes, Win! :) Thanks Saleem for all the hard work! Let's see how the build goes with the current version of IAS to see if we can close this bug (and open other, less critical ones). |
Hi Saleem, Indeed it does solves our problems in LLVMLinux with macros but we are still facing the same issues as you with the "defines" such as in: kernel/bounds.c:19:2: error: unexpected token at start of statement Regards, |
We seem to have implemented most of the assembly flags which the IAS wasn't able to cope, and now the problems we're seeing in the kernel are due to other issues with GCC/GAS, not unknown directives. The directives we haven't implemented are:
They will require substantial changes in the assembler and are not canonical. Furthermore, we need further evidence that they can't be avoided. The recent but that Saleem and Tinti found in the kernel has a new bug (#18891) and should be dealt with in there. This bug is now closed, thanks everyone for all the effort! |
mentioned in issue #19300 |
Extended Description
Here is a list of ARM assembly directives that the GNU assembler can handle but the Clang integrated assembler cannot. Note that these directives are not emitted by Clang itself when generating assembly files for ARM target.
The text was updated successfully, but these errors were encountered: