assigned to @rengolin

• .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?

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

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.

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.

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.s

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.

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.

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.

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).

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))
^
: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!

mentioned in issue #19300