This is an archive of the discontinued LLVM Phabricator instance.

[mips] Move expansion of .cpsetup to target streamer.
ClosedPublic

Authored by matheusalmeida on Apr 24 2014, 6:45 AM.

Details

Summary

There are two functional changes:

  1. The directive is not expanded for the ASM->ASM code path.
  2. If PIC is not set, there's no expansion for the ASM->OBJ code path (same behaviour as GAS).

Diff Detail

Event Timeline

matheusalmeida retitled this revision from to [mips] Move expansion of .cpsetup to target streamer..
matheusalmeida updated this object.
matheusalmeida edited the test plan for this revision. (Show Details)

The description is not entirely right... There are two functional changes:

  1. The directive is not expanded for the ASM->ASM code path
  2. If PIC is not set, there's no expansion for the ASM->OBJ code path.
dsanders added inline comments.Apr 25 2014, 2:55 AM
test/MC/Mips/cpsetup.s
31–32

Please re-add the check for the relocations

45–46

here too

55–62

I believe .cpsetup shouldn't emit any instructions in this case. If so, it would be more robust to add a label immediately after the directive and use CHECK-NEXT.

Added reloc information to the test now that it's possible to print it
with llvm-objdump.

The test has a FIXME because direct object emission for N32 does not work properly (yet).
At the moment we emit an ELF64 instead of an ELF32 and encode more than one operation in
a relocation record which is only valid for n64.

dsanders accepted this revision.Apr 30 2014, 7:07 AM
dsanders edited edge metadata.

LGTM

lib/Target/Mips/MipsTargetStreamer.h
144–145

I believe that the 'virtual' keyword should be dropped when 'override' is used

This revision is now accepted and ready to land.Apr 30 2014, 7:07 AM
matheusalmeida added inline comments.Apr 30 2014, 7:18 AM
lib/Target/Mips/MipsTargetStreamer.h
144–145

I saw a thread this morning discussing it and I forgot to update this patch. Thanks.

matheusalmeida edited edge metadata.
matheusalmeida updated this object.

Remove virtual keyword.

matheusalmeida closed this revision.May 1 2014, 3:31 AM