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. * .arch_extension * .dn * .qn * .even * .extend * .inst * .ldbouble * .movsp * .object_arch * .packed * .personalityindex * .pool * .thumb_set * .tlsdescseq * .unwind_raw
* .end
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?
Created attachment 11750 [details] Self-contained assembly file with various things clang can't handle Some of the attached code comes from GCC's -S output. I'd call those a priority because when you hit it you can't simply edit the source. There may be more of those that I haven't discovered, but they're easy to hunt for. * synonyms for vpush/vpop * ldrd/strd with only one explicit register argument * macros taking named parameters * immediate arithmetic using labels * named eabi_attribute argument * synonym for .rept (.rep) * bkpt with default immediate 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
* macros taking named parameters * immediate arithmetic using labels * synonym for .rept (.rep) Do not belong on this bug. These are generic IAS issues, not particular to ARM. * bkpt with default immediate 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: * macros taking named parameters * .arch_extension * .dn * .qn * .extend * .ldbouble * .movsp * .object_arch * .packed * .pool * .thumb_set * .tlsdescseq In review: * synonyms for vpush/vpop EHABI: * .personalityindex * .unwind_raw
(In reply to comment #23) > Just to make sure we're up-to-date on the progress, what's still missing: > > Not started: > * macros taking named parameters I've started looking at this. > * .arch_extension > * .dn > * .qn > * .extend > * .ldbouble > * .movsp Shouldnt this be part of EHABI? I also have a WIP patch for this. > * .object_arch > * .packed > * .pool SVN r197787 > * .thumb_set I started looking at this, but never finished it as I ran into some trouble with proper behaviour for undefined symbols. > * .tlsdescseq > > In review: > * synonyms for vpush/vpop > > EHABI: > * .personalityindex I have a partially correct patch for this, that I will try to finish up once I get the llvm-readobj stuff fixed. > * .unwind_raw
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: * macros taking named parameters In review: * synonyms for vpush/vpop * .personalityindex TODO, by type: Architecture definition: * .arch_extension * .object_arch Thumb function alias: * .thumb_set SIMD aliases: * .dn * .qn * .unreq Floating point support: * .extend * .ldbouble * .packed Exception Handling: * .movsp * .unwind_raw Thread support: * .tlsdescseq
.unreq is already supported by the parser. Work in progress: * macros taking named parameters * .movsp * .thumb_set In review: * synonyms for vpush/vpop * .personalityindex TODO, by type: Architecture definition: * .arch_extension * .object_arch SIMD aliases: * .dn * .qn Floating point support: * .extend * .ldbouble * .packed Exception Handling: * .unwind_raw Thread support: * .tlsdescseq
> .unreq is already supported by the parser. 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: * macros taking named parameters * .thumb_set * .unwind_raw In review: * synonyms for vpush/vpop * .personalityindex * .movsp TODO, by type: Architecture definition: * .arch_extension * .object_arch SIMD aliases: * .dn * .qn [will involve dealing with .unreq] Floating point support: * .extend * .ldbouble * .packed Thread support: * .tlsdescseq
Work in progress: * macros taking named parameters * .thumb_set In review: * .movsp * .object_arch * .personalityindex * .unwind_raw TODO, by type: Architecture definition: * .arch_extension SIMD aliases: * .dn * .qn [will involve dealing with .unreq] Floating point support: * .extend * .ldbouble * .packed Thread support: * .tlsdescseq
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: * .thumb_set TODO, by type: Macro Assembler: * named parameters Architecture definition: * .arch_extension SIMD aliases: * .dn * .qn [will involve dealing with .unreq] Floating point support: * .extend * .ldouble * .packed
.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: * .thumb_set TODO, by type: Macro Assembler: * named parameters SIMD aliases: * .dn * .qn [will involve dealing with .unreq] Floating point support: * .extend * .ldouble * .packed
I'm having a look at .dn/.qn aliases.
(In reply to comment #32) > 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.
> They are fairly tricky as you can end up changing mnemonics based upon the aliasing. 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. > Laned aliases are also slightly problematic with the current structure in a couple of different places. 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. > Storage of the data type indicators cause some problems as well. 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.
(In reply to comment #34) > 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).
Created attachment 12034 [details] alias.s
Created attachment 12035 [details] alias-diagnostics. Also, Im not sure if the tests are sufficient or complete, this was just what I came up with when playing around to figure out the full extent of the supported syntax.
Created attachment 12045 [details] starting patch, covering the basic cases Initial implementation of the dn/qn aliases, dealing with simple cases. Missing features: * apply the saved type on the instruction * carry the lane info into the register * parse the register alias again for things like x.s16 * implement expressions, not just register names * more error checks 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.
Created attachment 12051 [details] kernel build log with -k and integrated assembler This file shows the errors of compiling the ARM kernel with the integrated assembler and running with -k. Not that it will be only that, but macros seem to be the primary concern at the moment.
(In reply to comment #39) > 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. 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 think its fine to priortise other things first; Ive been doing that constantly. 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: 1. It's *just an alias* to a UAL directive/instructions/construct, or it's so simple to add that it doesn't make sense to *not* add. Most constructs you've added were in this category, and they were all very welcomed. 2. It's extensively used in critical user code (like the kernel, heavily used libraries, infrastructure software, etc) which *cannot* change their code because they not only use it extensively, but the alternatives are less desirable for technical reasons. Macro support falls here. 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. > Modern code does use it, and understandably so (skia is one example). 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. Second: gcc will not perform assembly validation and accept GARBAGE! 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).
(In reply to comment #47) > 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 DEFINE(NR_PAGEFLAGS, __NR_PAGEFLAGS); ^ include/linux/kbuild.h:5:25: note: expanded from macro 'DEFINE' asm volatile("\n->" #sym " %0 " #val : : "i" (val)) ^ <inline asm>:2:1: note: instantiated into assembly here ->NR_PAGEFLAGS #22 __NR_PAGEFLAGS ^ Regards, Tinti
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: * .dn/.qn * .extend/.ldbouble/.packed 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!