Implementing the ldr pseudo instruction in ARM integrated assembler

Project Infrastructure LLVM Dev List Archives
1

In an earlier email[1] I proposed adding support for the ldr
pseud-instruction to the ARM integrated assembler. After some discussion the
overall consensus seemed to be that it was worth adding. One concern was
that we needed to have adequate testing. I promised to provide more details
on what the behavior should be and provide some tests before starting the
implementation. The FileCheck-ified tests are attached to this email. They
currently pass when using gcc to assemble the test.

Here are the specific behaviors that are checked in the test:

1. Check that constants that fit into imm12 are converted to mov
    ldr r0, =0x3
2. Check that large constants are converted to ldr from constant pool
    ldr r0, =0x103
3. Duplicate constants should be merged to the same constant pool location
    ldr r0, =0x103
    ...
    ldr r0, =0x103
4. A section defined in multiple pieces is merged and uses a single constant
pool
    For example,
    
    .section e, "ax", %progbits
      f6:
        ldr r0, =0x103
    .section f, "ax", %progbits
      f7: add r0, r0, r0
    .section e, "ax", %progbits
      f8:
        add r0, r0, r0
        ldr r0, =0x105

    should only produce one constant pool for section e.

5. Check that symbols can be loaded using ldr pseudo
  ldr r0, =foo

6. Check that ldr pseudo can be used with expressions
  ldr r0, =0x101+6
  ldr r0, =bar+4

7. Check that it is used correctly in a macro
    .macro useit_in_a_macro
      ldr r0, =0x101
      ldr r0, =bar
    .endm
    .section k, "ax", %progbits
    f14:
      useit_in_a_macro

8. Check that an error is issued when the constant pool would be placed too
far away
  ldr r0, =0x101
  @... lots of instructions
  Error: invalid literal constant: pool needs to be closer

I have not yet looked into the code to see where/how this feature can be
implemented. Any pointers or feedback is welcome. I've filed a Bug[2] to
track this issue.

[1]: http://lists.cs.uiuc.edu/pipermail/llvmdev/2013-October/066808.html
[2]: http://llvm.org/bugs/show_bug.cgi?id=17769

-- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted
by The Linux Foundation

ldr_pseudo.s (4.23 KB)

2

Hi David,

In these examples, I don’t see the directive that indicates where the assembler should place the constant pool?

-Jim

3

Hi David,

8. Check that an error is issued when the constant pool would be placed too
far away

I'd say this one is actually the most involved constraint but there
don't actually seem to be any tests in the attached file for it.

And I believe the directive Jim's referring to is ".ltorg". It's
presumably going to have some interesting quirks of its own.

Cheers.

Tim.

4

Hi Jim,

In these examples, I don't see the directive that indicates where the
assembler should place the constant pool?

As Tim mentioned, the directive in question is .ltorg. I had planned to
implement that in a separate step once the initial support for ldr-pseudo
was complete. The .ltorg directive is only required when the constant pool
would be placed too far away for the offset to be encoded in the pc-relative
load. It gives the programmer an escape, but is not required for an initial
implementation.

I was thinking that without the .ltorg directive the constant pool would go
at the end of the section.

5

Hi Tim,

> 8. Check that an error is issued when the constant pool would be
> placed too far away

I'd say this one is actually the most involved constraint but there don't
actually seem to be any tests in the attached file for it.

I put the test in a separate file and forgot to attach it earlier. I've
attached it to this email. I can't say how difficult this will be to
implement, but Jim also indicated it could be tricky so I believe you both
:). I only have a simple test for this now, but I'd welcome suggestions on
improving it or any hits on the implementation.

And I believe the directive Jim's referring to is ".ltorg". It's
presumably going to have some interesting quirks of its own.

Yes, I replied to Jim saying that I was planning to implement .ltorg in a
separate step.

ldr_pseudo_errors.s (17.2 KB)

6

> I was thinking that without the .ltorg directive the constant pool
> would go at the end of the section.
>
So where does the assembler place the constant pool(s) if that directive
isn't present? I was under the impression it was always required.

From my understanding it is not required. I see that GCC will place it at

the end of the section. I don't know if it will ever place it anywhere
besides the end of the section when there is no .ltorg directive.

Here is the relevant section from the gcc docs
(ARM Directives (Using as)):

"""
.ltorg
This directive causes the current contents of the literal pool to be dumped
into the current section (which is assumed to be the .text section) at the
current location (aligned to a word boundary). GAS maintains a separate
literal pool for each section and each sub-section. The .ltorg directive
will only affect the literal pool of the current section and sub-section. At
the end of assembly all remaining, un-empty literal pools will automatically
be dumped.
"""

7

What does ARM’s documentation say?

8

>>> I was thinking that without the .ltorg directive the constant pool
>>> would go at the end of the section.
>>>
>> So where does the assembler place the constant pool(s) if that
>> directive isn't present? I was under the impression it was always
required.
>
> From my understanding it is not required. I see that GCC will place it
> at the end of the section. I don't know if it will ever place it
> anywhere besides the end of the section when there is no .ltorg
directive.
>
> Here is the relevant section from the gcc docs
> (ARM Directives (Using as)):
>
> """
> .ltorg
> This directive causes the current contents of the literal pool to be
> dumped into the current section (which is assumed to be the .text
> section) at the current location (aligned to a word boundary). GAS
> maintains a separate literal pool for each section and each
> sub-section. The .ltorg directive will only affect the literal pool of
> the current section and sub-section. At the end of assembly all
> remaining, un-empty literal pools will automatically be dumped.
> """
>

What does ARM's documentation say?

The ARM documentation says that the assembler puts the current literal pool
at the end of every code section, where the sections are determined by the
AREA directive or the end of the assembly. If the default literal pool will
be out of range the programmer can use the LTORG directive to assemble the
current literal pool immediately.

9

I put the test in a separate file and forgot to attach it earlier. I've
attached it to this email.

The ".space" directive could be very useful in making the test more manageable.

Other than that I'd be wary of instructions that might be relaxed
during object emission and suddenly make a load out of range. LLVM
seems to do this for Bcc, pc-relative loads, ADR and B. It's getting
towards implementation details but you'll want to bear it in mind when
adding tests for any implementation.

One question that's occurred is can this support PIC when you
reference other sections? That may well affect the relocation used.

ARM's own assembler may be able to (it certainly has --apcs=...
options that can tell it it's compiling PIC code). I can't recall a
similar command-line option to llvm-mc, but Clang-as-assembler
probably accepts -fPIC.

Goodness knows whether we want to do anything with it though.

(from Jim)

What does ARM’s documentation say?

http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0473c/Bgbbfgia.html

Fairly similar, though it doesn't seem to use a MOV opportunistically.

Tim.

10

Well, they’re consistent at least, so that’s good. That’s also pretty well-defined. I was afraid there was going to be some requirement that the assembler try to analyze things and figure out where good places were (a-la the constant island pass). Glad to hear that’s not the case.

There’s still a problem for Darwin, or any other platform that use subsections-via-symbols type layout tricks, though. There’s no assembler-time way to know how far apart the atoms in the section will be at runtime, as the linker can, and will, move things around.

The quick thought would be to emit them when the next atom begins, but that’ll fall over due to the typical “.align” which precedes the next function. The constant pool for the previous function would end up being emitted after the alignment directive for the following function, which will make that next symbol potentially not sufficiently aligned. For example,
_foo:
  ldr, r1, =0x12345678
  …
  bx lr

.align 4
_bar:
  …

The result will be that _bar is not 16-byte aligned, but only 4-byte aligned, which will come as quite the surprise to the programmer.

The next thought is to detect subsections-via-symbols and require the directive if another atom is seen and there is a non-empty constant pool. That gets a bit of chicken-and-egg, though, as the subsections-via-symbols directive is typically the last line of the .s file.

We could, perhaps, always require an explicit directive for all constant pools when using subsections-via-symbols and add a diagnostic check at the end of parsing (when we’re spitting out the non-empty pools) to see if there was a subsections-via-symbols directive in there anywhere.

Anyways, the main point of all of this is to reinforce the “there be dragons here” nature of this feature. It interacts with other parts of the assembler and the underlying assumptions of the platform in interesting ways. Lots of *really* careful test cases will be necessary.

11

The ".space" directive could be very useful in making the test more
manageable.

Ah, yes that is a great suggestion. Thank you!

Other than that I'd be wary of instructions that might be relaxed during
object emission and suddenly make a load out of range. LLVM seems to do
this for Bcc, pc-relative loads, ADR and B. It's getting towards
implementation details but you'll want to bear it in mind when adding
tests for any implementation.

Ok, thanks I will keep that in mind. Sounds like it could be tricky to write
a test for it, but I will try.

One question that's occurred is can this support PIC when you reference
other sections? That may well affect the relocation used.

ARM's own assembler may be able to (it certainly has --apcs=...
options that can tell it it's compiling PIC code). I can't recall a
similar command-line option to llvm-mc, but Clang-as-assembler probably
accepts -fPIC.

Goodness knows whether we want to do anything with it though.

Hmm, yes I had not considered that. It's a good question.

(from Jim)
> What does ARM's documentation say?

Documentation – Arm Developer
gia.html

Fairly similar, though it doesn't seem to use a MOV opportunistically.

I did notice the use of movw by armasm when comparing to gcc. I never did
observe gcc using movw and I was planning to follow that for the first step.
I think using movw would be a useful follow-on optimization.

12

There's still a problem for Darwin, or any other platform that use
subsections-via-symbols type layout tricks, though. There's no assembler-
time way to know how far apart the atoms in the section will be at
runtime, as the linker can, and will, move things around.

Hmm, yes that does sound quite tricky. How do we currently deal with that
for other pc-relative loads. Say the programmer writes something like this

_foo:
  ldr r1, [pc, #392]
_bar:
  .space 200
_baz:
  .space 200
_some_important_constants: .word 0x12345678

If bar and baz get deleted by the linker the offset would obviously be
wrong. Do we do anything special for this or just rely on the programmer not
to write this code?

We could, perhaps, always require an explicit directive for all constant
pools when using subsections-via-symbols and add a diagnostic check at the
end of parsing (when we're spitting out the non-empty pools) to see if
there was a subsections-via-symbols directive in there anywhere.

This seems to be a reasonable choice. It seems it would still be difficult
to detect all the cases where we could run into trouble. For example,
something like

_foo:
  ldr r1, =0x12345678
_bar:
  ...
.ltorg

If bar gets deleted we would still have the wrong offsets.

Anyways, the main point of all of this is to reinforce the "there be
dragons here" nature of this feature. It interacts with other parts of the
assembler and the underlying assumptions of the platform in interesting
ways. Lots of *really* careful test cases will be necessary.

Yes I see your point. Thanks for brining .subsections_via_symbols to my
attention.

13

There's still a problem for Darwin, or any other platform that use
subsections-via-symbols type layout tricks, though. There's no assembler-
time way to know how far apart the atoms in the section will be at
runtime, as the linker can, and will, move things around.

Hmm, yes that does sound quite tricky. How do we currently deal with that
for other pc-relative loads. Say the programmer writes something like this

_foo:
  ldr r1, [pc, #392]
_bar:
  .space 200
_baz:
  .space 200
_some_important_constants: .word 0x12345678

If bar and baz get deleted by the linker the offset would obviously be
wrong. Do we do anything special for this or just rely on the programmer not
to write this code?

That code is basically undefined behaviour with subsections-via-symbols. PC-relative loads can’t cross atoms. This is why, for example, that all labels inside a function must be assembler-local labels. You’ll see all sorts of “interesting” code sequences and relocation tricks for pc-relative stuff.

We could, perhaps, always require an explicit directive for all constant
pools when using subsections-via-symbols and add a diagnostic check at the
end of parsing (when we're spitting out the non-empty pools) to see if
there was a subsections-via-symbols directive in there anywhere.

This seems to be a reasonable choice. It seems it would still be difficult
to detect all the cases where we could run into trouble. For example,
something like

_foo:
ldr r1, =0x12345678
_bar:
...
.ltorg

If bar gets deleted we would still have the wrong offsets.

Yeah, that would need to be illegal as well. Fun times…

It sounds like this is getting constrained to a pretty reasonable set of features for the assembler to deal with. It’ll definitely be a good usability thing and will make lots of existing code a lot happier with clang, which is a good thing. Thanks for taking on the challenge and working on it.

-Jim

14

I have attached an initial patch that implements the ldr pseudo. It still
needs some clean up and more tests, but I would like some feedback on the
approach I used and if there are any objections to implementing it this way.
Here is my approach:

Add a finishParse() callback to the target asm parser
  This callback is invoked when the parse has finished
  successfully. It will be used to write out constant pools
  for the ldr pseudo.

LDR pseudo implementation
  Keep a map from Section => ConstantPool
  When parsing ldr r0, =val
    parse val as an MCExpr
    get ConstantPool for current Section
    remember val in ConstantPool (at next available Offset)
    add operand to ldr that is MCExpr of (ConstantPool.Label + Offset)

  On finishParse() callback
    Write out all non-empty constant pool vals to the end of their sections

Missing features
  1. Does not convert load of small constants to mov
     (e.g. ldr r0, =0x1 => mov r0, 0x1)
  2. Does reuse constant pool entries for same constant

It currently passes the tests I shared earlier (including the error message)
except that ldr is not yet converted to mov when possible. Please let me
know if you have any issues with this method of implementation.

Thanks,
-David

-- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted
by The Linux Foundation

0001-PR17769-Initial-implementation-of-ldr-pseudo.patch (11.1 KB)

15

Hi David,

Thanks for your efforts here. I have a few comments on your patch, although
I realise it's still a work in progress.

+class ConstantPool {
+ MCSymbol *Label;
+ typedef std::vector<const MCExpr*> EntryVecTy;

Use a SmallVector here?

+ MCSymbol *getLabel() {return Label;}
+ size_t getNumEntries() {return Entries.size();}
+ const MCExpr *getEntry(size_t Num) {return Entries[Num];}
These can be const.

+ int64_t ConstantPoolCounter;
This can be unsigned.

+ case AsmToken::Equal: {
+ const MCSection *Section =
getParser().getStreamer().getCurrentSection().first;
+ assert(Section);
We should probably check here that the mnemonic is actually 'ldr', e.g. see
the AsmToken::Identifier case.

+@ RUN: clang -target arm -integrated-as -c %s -o %t1 2> %t2; echo "ok"
+@ RUN: cat %t2 | FileCheck %s
Clang tests shouldn't be in the LLVM regression suite. Use llvm-mc instead
for assembling.

I'm not very familiar with the code around the asm parser, so I expect more
detailed comments from others to follow.

Cheers,
Amara

16

Hi Amara,

Thanks for your suggestions. I have made the changes you suggested and added
a new test to check that we print an error when parsing a non-ldr mnemonic
with an operand containing `=`. The updated patch is attached.

-- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted
by The Linux Foundation

0001-PR17769-Initial-implementation-of-ldr-pseudo.patch (12.6 KB)

17

Moving discussion to llvm-commits now that I have a more developed
implementation:
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20131111/195401.
html

18

Hi David,

Maybe I’m just blind, but where’s the code to handle the .ltorg directive? Is that a separate patch, maybe? Without that, this is not going to be usable in any circumstance using subsections-via-symbols.

+typedef std::map<const MCSection *, ConstantPool> ConstantPoolMapTy;

This feels odd to me. Can you elaborate a bit more on the data structure choices?? I would have expected constants to be grouped together explicitly by section and then a nested loop over those sections instead. As-is, won’t this potentially cause lots of switching back and forth between sections at the end of assembly?

+ MCSymbol *Label =
+ getContext().GetOrCreateSymbol("$cp." + Twine(ConstantPoolCounter++));

As previously mentioned, this sort of label name isn’t portable. Whether ‘$’ is legal isn’t guaranteed, and there’s no assembler-local label prefix.

+ ConstantPoolMapTy::iterator Cp = ConstantPools.find(Section);

Minor detail in various places in the code. Acronyms should be capitalized, so “CP” instead of “Cp”. Even better would be spelled out names if they’re short enough to make sense in the context of the code.

+void ARMAsmParser::finishParse() {
+ for (ConstantPoolMapTy::iterator CpI = ConstantPools.begin(), CpE = ConstantPools.end(); CpI != CpE; ++CpI) {
+ const MCSection *Section = CpI->first;
+ ConstantPool &CP = CpI->second;
+ MCStreamer &Streamer = getParser().getStreamer();
+ Streamer.SwitchSection(Section);
+ Streamer.EmitLabel(CP.getLabel());
+ for (size_t I = 0; I < CP.getNumEntries(); ++I)
+ Streamer.EmitValue(CP.getEntry(I), 4);
+ }
+}

This is missing data-in-code indicators (see EmitDataRegion()).

+@RUN: clang -target arm -integrated-as -c %s -o %t
+@RUN: llvm-objdump -d %t | FileCheck %s
+@RUN: llvm-objdump -r %t | FileCheck --check-prefix=RELOC %s

MC tests should not call clang. There is no guarantee that clang will be present on the system or being built in the current LLVM build tree. Use llvm-mc instead. Likewise, there shouldn’t be any need to output an object file. Just checking the .s output from the asm streamer should be sufficient here.

These tests are very linux specific, including names and relocation type information. What happens when compiling on or for other platforms?

-Jim

19

Hi Jim,

Thanks for the review. It seems like you were looking at an old patch. I've
attached the latest patches to this email (and a squashed version of all
three for easy reading). I believe many of your concerns were addressed. See
below for a detailed response.

Maybe I'm just blind, but where's the code to handle the .ltorg directive?

It is implemented in patch 0003 in this email.

+typedef std::map<const MCSection *, ConstantPool> ConstantPoolMapTy;

This feels odd to me. Can you elaborate a bit more on the data structure
choices?? I would have expected constants to be grouped together
explicitly by section and then a nested loop over those sections instead.
As-is, won't this potentially cause lots of switching back and forth
between sections at the end of assembly?

I just added this comment to the code to try and address your question:

// Map type used to keep track of per-Section constant pools used by the
// ldr-pseudo opcode. The map associates a section to its constant pool. The
// constant pool is a vector of (label, value) pairs. When the ldr
// pseudo is parsed we insert a new (label, value) pair into the constant
pool
// for the current section and add MCSymbolRefExpr to the new label as
// an opcode to the ldr. After we have parsed all the user input we
// output the (label, value) pairs in each constant pool at the end of the
// section.
typedef std::map<const MCSection *, ConstantPool> ConstantPoolMapTy;

Basically it groups the constants by section and writes them out at the end.
It will have to switch sections at the end of the assembly once per section
that has a constant pool.

+ MCSymbol *Label =
+ getContext().GetOrCreateSymbol("$cp." +
+ Twine(ConstantPoolCounter++));

As previously mentioned, this sort of label name isn't portable. Whether
'$' is legal isn't guaranteed, and there's no assembler-local label
prefix.

This was fixed in a later patch by using the CreateTempSymbol() API in
MCContext.

+ ConstantPoolMapTy::iterator Cp = ConstantPools.find(Section);

Minor detail in various places in the code. Acronyms should be
capitalized, so "CP" instead of "Cp". Even better would be spelled out
names if they're short enough to make sense in the context of the code.

I will clean that up when I rebase the changes.

+void ARMAsmParser::finishParse() {
+ for (ConstantPoolMapTy::iterator CpI = ConstantPools.begin(), CpE =
ConstantPools.end(); CpI != CpE; ++CpI) {
+ const MCSection *Section = CpI->first;
+ ConstantPool &CP = CpI->second;
+ MCStreamer &Streamer = getParser().getStreamer();
+ Streamer.SwitchSection(Section);
+ Streamer.EmitLabel(CP.getLabel());
+ for (size_t I = 0; I < CP.getNumEntries(); ++I)
+ Streamer.EmitValue(CP.getEntry(I), 4);
+ }
+}

This is missing data-in-code indicators (see EmitDataRegion()).

I was not aware of that API, thanks for pointing it out. I added the calls
in the latest patch. See the ConstantPool::emitEntries() method.

+@RUN: clang -target arm -integrated-as -c %s -o %t
+@RUN: llvm-objdump -d %t | FileCheck %s
+@RUN: llvm-objdump -r %t | FileCheck --check-prefix=RELOC %s

MC tests should not call clang. There is no guarantee that clang will be
present on the system or being built in the current LLVM build tree. Use
llvm-mc instead.

That was fixed in a later patch.

Likewise, there shouldn't be any need to output an object
file. Just checking the .s output from the asm streamer should be
sufficient here.

I deliberately used an object file test because I wanted to make sure that
the correct offsets and relocations were being generated. Additionally, some
error tests require object code emission (to find out the constant pool
offset is too far away). If you feel strongly that they should be converted
to asm streamer tests I will do it.

These tests are very linux specific, including names and relocation type
information. What happens when compiling on or for other platforms?

Please see the darwin tests in the latest patch.

0001-Add-a-finishParse-callback-to-the-targer-asm-parser.patch (1.55 KB)

0002-Implement-the-ldr-pseudo-opcode-for-ARM-assembly.patch (23.5 KB)

0003-Implement-the-.ltorg-directive-for-ARM-assembly.patch (8.16 KB)

squashed.patch (32.3 KB)

20

Hi Jim,

Thanks for the review. It seems like you were looking at an old patch. I've
attached the latest patches to this email (and a squashed version of all
three for easy reading). I believe many of your concerns were addressed. See
below for a detailed response.

Ack! Sorry about that. That would certainly explain some of my confusion. I suspect some (mis)threading of emails got in the way. Thanks again for your patience working through this.

Responses to a few specific questions below, and I’ll follow up on the new patch separately.

Maybe I'm just blind, but where's the code to handle the .ltorg directive?

It is implemented in patch 0003 in this email.

Excellent.

+typedef std::map<const MCSection *, ConstantPool> ConstantPoolMapTy;

This feels odd to me. Can you elaborate a bit more on the data structure
choices?? I would have expected constants to be grouped together
explicitly by section and then a nested loop over those sections instead.
As-is, won't this potentially cause lots of switching back and forth
between sections at the end of assembly?

I just added this comment to the code to try and address your question:

// Map type used to keep track of per-Section constant pools used by the
// ldr-pseudo opcode. The map associates a section to its constant pool. The
// constant pool is a vector of (label, value) pairs. When the ldr
// pseudo is parsed we insert a new (label, value) pair into the constant
pool
// for the current section and add MCSymbolRefExpr to the new label as
// an opcode to the ldr. After we have parsed all the user input we
// output the (label, value) pairs in each constant pool at the end of the
// section.
typedef std::map<const MCSection *, ConstantPool> ConstantPoolMapTy;

Basically it groups the constants by section and writes them out at the end.
It will have to switch sections at the end of the assembly once per section
that has a constant pool.

+ MCSymbol *Label =
+ getContext().GetOrCreateSymbol("$cp." +
+ Twine(ConstantPoolCounter++));

As previously mentioned, this sort of label name isn't portable. Whether
'$' is legal isn't guaranteed, and there's no assembler-local label
prefix.

This was fixed in a later patch by using the CreateTempSymbol() API in
MCContext.

+ ConstantPoolMapTy::iterator Cp = ConstantPools.find(Section);

Minor detail in various places in the code. Acronyms should be
capitalized, so "CP" instead of "Cp". Even better would be spelled out
names if they're short enough to make sense in the context of the code.

I will clean that up when I rebase the changes.

+void ARMAsmParser::finishParse() {
+ for (ConstantPoolMapTy::iterator CpI = ConstantPools.begin(), CpE =
ConstantPools.end(); CpI != CpE; ++CpI) {
+ const MCSection *Section = CpI->first;
+ ConstantPool &CP = CpI->second;
+ MCStreamer &Streamer = getParser().getStreamer();
+ Streamer.SwitchSection(Section);
+ Streamer.EmitLabel(CP.getLabel());
+ for (size_t I = 0; I < CP.getNumEntries(); ++I)
+ Streamer.EmitValue(CP.getEntry(I), 4);
+ }
+}

This is missing data-in-code indicators (see EmitDataRegion()).

I was not aware of that API, thanks for pointing it out. I added the calls
in the latest patch. See the ConstantPool::emitEntries() method.

+@RUN: clang -target arm -integrated-as -c %s -o %t
+@RUN: llvm-objdump -d %t | FileCheck %s
+@RUN: llvm-objdump -r %t | FileCheck --check-prefix=RELOC %s

MC tests should not call clang. There is no guarantee that clang will be
present on the system or being built in the current LLVM build tree. Use
llvm-mc instead.

That was fixed in a later patch.

Likewise, there shouldn't be any need to output an object
file. Just checking the .s output from the asm streamer should be
sufficient here.

I deliberately used an object file test because I wanted to make sure that
the correct offsets and relocations were being generated. Additionally, some
error tests require object code emission (to find out the constant pool
offset is too far away). If you feel strongly that they should be converted
to asm streamer tests I will do it.

A bit of both. I feel strongly that there should be tests that don’t emit to object files, but do agree that there’s value to checking object as well (for the range checking in particular). In particular, the asm streamer tests can probably be platform independent, or at least close. The object file tests obviously cannot. For the relocations, is there something specific about the code generated here that tests those in ways that existing relocation tests don’t cover? I would have expected that part to be totally generic, as the pseudo is just a syntactic convenience after all and not expressing anything that can’t already be written in a .s file without the construct.

These tests are very linux specific, including names and relocation type
information. What happens when compiling on or for other platforms?

Please see the darwin tests in the latest patch.

Will do, and thanks again.

-Jim