This is an archive of the discontinued LLVM Phabricator instance.

[LLD][ELF][ARM] Replace assembler files with yaml
AbandonedPublic

Authored by psmith on Mar 22 2020, 2:52 PM.

Details

Summary

In the near future llvm-mc will resolve the fixups that generate R_ARM_THUMB_PC8 and R_ARM_THUMB_PC12 at assembly time (see comments in D72892), and forbid inter-section references. Change the LLD tests for these relocations to use yaml2obj to avoid LLD tests failing when this happens. The tests generate the same instructions, relocations and symbols, and have been edited from obj2yaml.

I will need to make equivalent changes for D75349 Arm equivalent relocations, but this is still in review so these don't need changing before llvm-mc.

Diff Detail

Event Timeline

psmith created this revision.Mar 22 2020, 2:52 PM

I think it is fine. When llvm-mc can't be used it is a job for yaml2obj.

I've investigated one of the test cases and put few comments about excessive YAML parts mostly.
These comments seems are general and can be applied to other tests added as well.

lld/test/ELF/arm-thumb-adr-err.test
40

Seems you do not need Flags.

45

And AddressAlign too (and in other places).

79

I'd add a comment for such magic values, it is unclear why 1020 and not 1019 fo example?

88

Doesn't seem you need .ARM.attributes section?

(Side note: seems we might want to teach yaml2obj about SHT_ARM_ATTRIBUTES..)

107

I'd suggest to move CHECK lines right below the line with FileCheck %s.

MaskRay added a comment.EditedMar 23 2020, 1:27 PM

I think it is fine. When llvm-mc can't be used it is a job for yaml2obj.

I've investigated one of the test cases and put few comments about excessive YAML parts mostly.
These comments seems are general and can be applied to other tests added as well.

yaml2obj tests do seem to be more verbose and opaque. I just remembered the .reloc directive (e.g. D61992). Ideally we could test with:

.globl target3
target3:

.reloc ., R_ARM_THM_PC12, foo
ldr r2, target3
# arm-linux-gnueabi-as assembles fine.

However, our use cases are:

  • we hope a MCFixupKind corresponding to a short-range relocation gets resolved at assembly time.
  • we hope a .reloc directive is emitted as is.

The current logic can not satisfy the two goals simultaneously.

bool MCObjectStreamer::emitRelocDirective(const MCExpr &Offset, StringRef Name,
                                          const MCExpr *Expr, SMLoc Loc,
                                          const MCSubtargetInfo &STI) {
  // Name ("R_ARM_THM_PC12") is converted to a MCFixupKind
  Optional<MCFixupKind> MaybeKind = Assembler->getBackend().getFixupKind(Name);

Then, ARMAsmBackend::getFixupKindInfo thinks this MCFixupKind should be resolved at assembly time.

The observation suggests to me that getFixupKind is not suitable and we should probably add mechanism to use an arbitrary relocation type (for example, a value larger than FirstLiteralRelocationKind is considered to have a relocation number of V-FirstLiteralRelocationKind)

Thanks for the comments, I'll do my best to trim down the yaml and resubmit tomorrow.

MaskRay added a comment.EditedMar 24 2020, 7:53 PM

Thanks for the comments, I'll do my best to trim down the yaml and resubmit tomorrow.

Wait. Sorry that I only recalled the .reloc directive after you had started to work on yaml2obj tests. In an assembly test, a .section directive corresponds to several lines in a YAML. A symbol requires one or two lines in an assembly while it may require up to 5 lines in a YAML.

I created D76746 to generalize .reloc. With this patch, we can probably write:

.globl bar
bar:
adr r0, bar; .reloc 0, R_ARM_ALU_PC_G0, bar

Thanks for the comments, I'll do my best to trim down the yaml and resubmit tomorrow.

Wait. Sorry that I only recalled the .reloc directive after you had started to work on yaml2obj tests. In an assembly test, a .section directive corresponds to several lines in a YAML. A symbol requires one or two lines in an assembly while it may require up to 5 lines in a YAML.

I created D76746 to generalize .reloc. With this patch, we can probably write:

.globl bar
bar:
adr r0, bar; .reloc 0, R_ARM_ALU_PC_G0, bar

I think it will be close. As the adr and ldr are pseudo instructions I think they will create a fixup anyway, so we'll need something like:

.global bar
fake: adr, r0, fake // fixup resolves at assembly time to the pc-bias
.reloc 0, R_ARM_ALU_PC_G0, bar // relocation created to be resolved at link time

Or use .inst, but this is still smaller than using .yaml.

psmith abandoned this revision.Apr 1 2020, 1:41 AM

Abandoned in favour of D77200 which uses .inst and .reloc

lld/test/ELF/arm-thumb2-ldrlit-err.test