Page MenuHomePhabricator

[M68k] (Patch 6/8) IR Tests
AcceptedPublic

Authored by myhsu on Sep 27 2020, 10:22 PM.

Details

Summary

All of the regression (IR) tests

Diff Detail

Event Timeline

myhsu created this revision.Sep 27 2020, 10:22 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 27 2020, 10:22 PM
myhsu requested review of this revision.Sep 27 2020, 10:22 PM
myhsu updated this revision to Diff 295466.Sep 30 2020, 10:14 PM
myhsu retitled this revision from [M68K] (Patch 6/8) MC supports and assembly to [M68K] (Patch 6/8) IR Tests.
myhsu edited the summary of this revision. (Show Details)
myhsu added reviewers: jrtc27, hiraditya.

Part of the restructing of this patch series. Now all of the tests are centralized to this patch

Good to have these tests. I'd wait for the previous patches in the stack to be reviewed.

myhsu updated this revision to Diff 302186.Nov 1 2020, 4:30 PM
myhsu retitled this revision from [M68K] (Patch 6/8) IR Tests to [M68k] (Patch 6/8) IR Tests.

[NFC] Rename M680x0 to M68k

Nice set of tests, well separated, not a lot of pattern-match on the FileCheck side, which is good, asm and object emission.

No idea about the correctness wrt m68k, though. :)

Will the extract-section util work on windows buildbots?

I suggest you use update_llc_test_checks.py to automatically generate the CHECK lines for the IR tests (see the RISC-V backend which does this pretty diligently). Otherwise it's a real nuisance to update them after CodeGen changes that affect tests.

myhsu added a comment.Nov 30 2020, 9:42 PM

Will the extract-section util work on windows buildbots?

We haven't tried that on Windows unfortunately, I believe our buildbot is only using Linux and maybe BSD now.

myhsu added a comment.Nov 30 2020, 9:53 PM

I suggest you use update_llc_test_checks.py to automatically generate the CHECK lines for the IR tests (see the RISC-V backend which does this pretty diligently). Otherwise it's a real nuisance to update them after CodeGen changes that affect tests.

Good point. However few tests are still failing, I guess we can't run this script to update the tests before we fix those right?

myhsu updated this revision to Diff 309424.Dec 3 2020, 5:54 PM
  • Fix / Update failed test cases. Now we have 100% passing rate except one XFAIL, yeah!
    • Most of the previous failed test cases were caused by less-optimal results from previous version of codegen. For example, the codegen for jump tables.
    • The only XFAIL is caused by (selection DAG) ISel which failed to select one instruction that supposed to have 16-bit operand but a 8-bit one was given. It seems to take more time to fix it so just mark XFAIL for now
myhsu updated this revision to Diff 309791.Dec 6 2020, 1:08 PM
  • Update test cases related to 8-bit multiplications.

Will the extract-section util work on windows buildbots?

We haven't tried that on Windows unfortunately, I believe our buildbot is only using Linux and maybe BSD now.

Is there anyway that you can replace the extract-section bash script with a extract-section.py python implementation? That should simplify windows support in the future.

myhsu updated this revision to Diff 310754.Dec 9 2020, 8:41 PM
  • Rewrite utils/extract-section into utils/extract-section.py
    • The new Python script is also more generic which doesn't have some M68k specific assumption (e.g. always group in 16-bits in hex mode) as the previous version
  • Update M68k-local LIT config
MaskRay added inline comments.Dec 14 2020, 7:19 PM
llvm/utils/extract-section.py
1

If not used, please drop it.

llvm-objcopy --dump-section=.foo=file a.o /dev/null

myhsu added inline comments.Dec 15 2020, 12:56 PM
llvm/utils/extract-section.py
1

If you're referring to this script as a whole, it is used extensively in M68k's test suite.

This script is designed for printing certain section in textual format. So I'm afraid your llvm-objcopy command might not be sufficient. As the comment below described, this script is similar to llvm-readobj but the latter only support one output format (i.e. hex with lots of redundant info like ASCii decoding), where in many cases, we want other formats like hex string with different width and textual bits.

RKSimon added inline comments.Dec 17 2020, 3:46 AM
llvm/test/CodeGen/M68k/ASM/Alloc/dyn_alloca_aligned.ll
1

Very pedantic - but its more common just to use the default check prefix (CHECK) for a single run instead of a custom (x00) one.

rengolin accepted this revision.Dec 18 2020, 4:08 AM

This LGTM now, thanks!

I agree with Simon that using the default CHECK in FileCheck is more common and easier to read than x00-NEXT.

However, I imagine this is like that because there would be other tests, for other sub-architectures (x10, x20, ...) that would be tested on the same file and therefore would need special CHECK lines on their own.

If that's true, than it should be fine to keep it like this for now, even if there's only one. Otherwise, you'd have to regex-replace all files now, and then later again, which is wasteful.

This revision is now accepted and ready to land.Dec 18 2020, 4:08 AM

I see very little by way of testing the return side of the calling convention, and nothing for sret.

llvm/test/CodeGen/M68k/ASM/Alloc/dyn_alloca_aligned.ll
1

Especially not one that's got lowercase letters in it...

llvm/test/CodeGen/M68k/ASM/Arith/imul.ll
93

You don't mean that...

llvm/test/CodeGen/M68k/ASM/CConv/c-call.ll
15

Do you need uwtable on any of these?

RKSimon added inline comments.Mon, Dec 21, 1:41 AM
llvm/test/CodeGen/M68k/ASM/Arith/imul.ll
1

why are you setting -O0 here?