Page MenuHomePhabricator

[MC] Support labels as offsets in .reloc directive
ClosedPublic

Authored by vstefanovic on Nov 1 2018, 12:38 PM.

Details

Summary

Currently, expressions like

.reloc 1f, R_MIPS_JALR, foo

1: nop

are not allowed, ie. an offset in .reloc can only be absolute value.
This patch adds support for labels as offsets.
If offset is a forward declared label, MCObjectStreamer keeps the fixup locally
and adds it to the fixups vector after the label (and its offset) is defined.
label+number is not supported yet.

Diff Detail

Repository
rL LLVM

Event Timeline

vstefanovic created this revision.Nov 1 2018, 12:38 PM

With this patch I get the following failed test cases.

Clang :: CodeGen/asm-parser-info.S
Clang :: CodeGen/mips-inline-asm-abi.c
Clang :: CodeGen/thinlto-diagnostic-handler-remarks-with-hotness.ll
Clang :: CodeGen/thinlto-distributed-cfi-devirt.ll
Clang :: CodeGen/thinlto-distributed-cfi.ll
Clang :: CodeGen/thinlto-distributed.ll
Clang :: CodeGen/thinlto-multi-module.ll
Clang :: CodeGen/thinlto-split-dwarf.c
Clang :: CodeGen/thinlto_backend.ll
Clang :: Driver/clang-offload-bundler.c
Clang :: Driver/mips-mabs-warning.c
Clang :: Driver/relax.s
Clang :: Misc/cc1as-split-dwarf.s
Clang :: Modules/DebugInfoTransitiveImport.m
Clang :: Modules/ExtDebugInfo.cpp
Clang :: Modules/ModuleDebugInfo.cpp
Clang :: Modules/ModuleDebugInfo.m
Clang :: Modules/debug-info-moduleimport-in-module.m
Clang :: Modules/lsv-debuginfo.cpp
Clang :: Modules/module_file_info.m
Clang :: Modules/pch_container.m
Clang :: PCH/debug-info-pch-path.c

Sorry; thanks for noticing.

Init PendingFixup.Sym to nullptr.

vstefanovic edited the summary of this revision. (Show Details)

While we're at it, supported multiple pending fixups.

LGTM, nit below.

Please delay the commit a bit to get a chance other reviewers to take a look at the patch.

include/llvm/MC/MCObjectStreamer.h
43 ↗(On Diff #172616)

The class can be replaced by struct.

47 ↗(On Diff #172616)
  • Why don't use c++ style class fields initialization?
  • MCSym but McFixup. I think it's better to name either MCSym / MCFixup or McSym / McFixup.
lib/MC/MCObjectStreamer.cpp
658 ↗(On Diff #172616)

That can be written a bit more briefly:

PendingFixups.emplace_back(&SRE.getSymbol(), DF,
                           MCFixup::create(-1, Expr, Kind, Loc));
atanasyan accepted this revision.Nov 8 2018, 12:38 PM
This revision is now accepted and ready to land.Nov 8 2018, 12:38 PM
atanasyan added inline comments.Nov 8 2018, 12:42 PM
lib/MC/MCObjectStreamer.cpp
80 ↗(On Diff #172616)

One more nit: clear PendingFixups here like we clear PendingLabels in the flushPendingLabels.

It'd be really great if we could handle all of this in the mips backend rather than in general code.

-eric

.reloc directive (and setting its offset as label) isn't mips-only feature, GNU as supports it for all the architectures (or at least for x86, arm, ppc).
But if we don't want this in llvm for other arches than mips, we should move it to mips backend.

Addressed comments.

vstefanovic marked 4 inline comments as done.Nov 19 2018, 11:18 AM

I assume that we do want to keep this feature for all the architectures, if anyone disagrees, please shout.

Re-uploaded the patch with more context.

This revision was automatically updated to reflect the committed changes.