This is an archive of the discontinued LLVM Phabricator instance.

[M68k][test] Initial migration of MC tests
ClosedPublic

Authored by myhsu on Apr 27 2021, 4:24 PM.

Details

Summary

As the context depicted by bug 49865[1], we are migrating tests under
test/CodeGen/M68k/Encoding, which was originally used to test
instruction encoding using MIR file as input, into test/MC/M68k. We
are also adding test directives for AsmParser using the same set of
inputs.

Currently we are converting the original MIR test files into assembly
code as well as translating the original LIT "RUN" statement into one
that only uses built-in LLVM tools (i.e. Get rid of extract-section).

However, since AsmParser has not completely finished, many of these
original test cases fail. Thus, this patch only migrate test files
that are passed by the current implementation of AsmParser (and
MCCodeEmitter). The remaining tests (under test/CodeGen/M68k/Encoding)
will be ported alone with the patch that fixes the related issues.

[1]: https://bugs.llvm.org/show_bug.cgi?id=49865

Diff Detail

Event Timeline

myhsu requested review of this revision.Apr 27 2021, 4:24 PM
myhsu created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptApr 27 2021, 4:25 PM
rengolin added a subscriber: zixuan-wu.

Adding @zixuan-wu as a reviewer, who's also adding a new target and could help with the recently acquired expertise. :)

ricky26 accepted this revision.EditedMay 2 2021, 3:04 PM

LGTM!

The relocation tests look a little messier than the others (were they generated from the old tests?) but it's pretty clear and more representative of what LLVM will output I guess.

Everything is looking much tidier. 😄

This revision is now accepted and ready to land.May 2 2021, 3:04 PM
jrtc27 added a comment.May 2 2021, 3:09 PM

You should probably model your relocation tests on the RISC-V ones, those are a lot cleaner in style

llvm/test/MC/M68k/Arith/Classes/MxExt.s
4

None of these need symbols, just have the raw instructions unindented in the files

llvm/test/MC/M68k/Relocations/text-plt.s
6

Why are there auto-generated comments? You don't need .text. Why is the symbol all-caps, that's not idiomatic?

8

Don't mix comment styles, pick one and stick with it

myhsu added a comment.May 2 2021, 3:34 PM

LGTM!

The relocation tests look a little messier than the others (were they generated from the old tests?) but it's pretty clear and more representative of what LLVM will output I guess.

Yes they're brought manually from the old tests. And I agree with @jrtc27 that I should reformat relocation tests to only check for essential elements.

myhsu updated this revision to Diff 342297.May 2 2021, 6:05 PM
myhsu marked 3 inline comments as done.
  • Adopting better formats for relocation tests
  • Cleanup unused directives like '.text' and '.globl'
myhsu updated this revision to Diff 342298.May 2 2021, 6:13 PM

Use lowercase on some of the symbols

This revision was automatically updated to reflect the committed changes.
jrtc27 added inline comments.May 5 2021, 5:26 PM
llvm/test/MC/M68k/Arith/Classes/MxNEG.s
4

This file was committed with labels still

myhsu added inline comments.May 5 2021, 5:33 PM
llvm/test/MC/M68k/Arith/Classes/MxNEG.s
4

The labels here are used to group different categories of instructions, such that we can isolate failure in a finer granularity. Therefore in the last changes I only clean up files with only a single label.

jrtc27 added inline comments.May 5 2021, 5:34 PM
llvm/test/MC/M68k/Arith/Classes/MxNEG.s
4

That is entirely unnecessary. Other targets don't do this. Labels are only added when you need an actual label.

myhsu marked 2 inline comments as done.May 5 2021, 5:37 PM
myhsu added inline comments.
llvm/test/MC/M68k/Arith/Classes/MxNEG.s
4

okay, I will fix it shortly

myhsu marked an inline comment as done.May 5 2021, 5:37 PM
myhsu added inline comments.May 5 2021, 5:52 PM
llvm/test/MC/M68k/Arith/Classes/MxNEG.s
4
llvm/test/CodeGen/M68k/Encoding/Relocations/data-gotpcrel.mir