Page MenuHomePhabricator

[lld][WebAssembly] Convert some lld tests to assembly
ClosedPublic

Authored by sbc100 on May 20 2020, 10:57 PM.

Details

Summary

When we originally wrote these tests we didn't have a stable and
fleshed out assembly format. Now we do so we should prefer that
over llvm ir for lld tests to avoid including more part of llvm
than necessary in order to run the test.

This change converts just 30 out of about 130 test files. More to
come when I have some more time.

Diff Detail

Event Timeline

sbc100 created this revision.May 20 2020, 10:57 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 20 2020, 10:57 PM
sbc100 edited the summary of this revision. (Show Details)May 20 2020, 11:00 PM
sbc100 edited the summary of this revision. (Show Details)
sbc100 added a reviewer: aardappel.
sbc100 edited the summary of this revision. (Show Details)May 21 2020, 11:44 AM
aardappel accepted this revision.May 21 2020, 12:26 PM

Wow, impressive! Not sure if I am the best person to guarantee this tests the same things as the IR tests did, though.

This revision is now accepted and ready to land.May 21 2020, 12:26 PM
dschuff added inline comments.May 21 2020, 12:52 PM
llvm/test/MC/WebAssembly/blockaddress.ll
7 ↗(On Diff #265576)

do we have any tests that test the lowering of the blockaddress IR instruction?

dschuff added inline comments.May 21 2020, 12:57 PM
lld/test/wasm/optional-symbol.s
3

pre-exsting issue, but should this test have an expectation?

lld/test/wasm/signature-mismatch.ll
79

is this change in expectation intended?

dschuff accepted this revision.May 21 2020, 1:09 PM

What is good documentation to read to become familiar with all the generic assembly directives? I could write a .ll test from scratch by hand, but I don't know the .s format well enough to write an assembly test from scratch. I would like to avoid my workflow becoming 1) write test as .ll then 2) run llc to get .s and commit the output.

lld/test/wasm/Inputs/call-indirect.s
29

Do I understand correctly that the .int32 foo declares a symbol foo and .functype foo () -> (i32) makes it a function symbol with that type?

sbc100 marked 3 inline comments as done.May 21 2020, 2:09 PM

What is good documentation to read to become familiar with all the generic assembly directives? I could write a .ll test from scratch by hand, but I don't know the .s format well enough to write an assembly test from scratch. I would like to avoid my workflow becoming 1) write test as .ll then 2) run llc to get .s and commit the output.

I don't know if there is any specific docs for this format. Its shared a *lot* of code with the gerneric asm parser / printer so a lot of the format matches what you might see in any .s file (e.g. x86).

I'm afraid that the technique of running llc to discover the format is probably the best way to go today. At least then its guaranteed to be current. Sorry I don't have a better answer.

lld/test/wasm/optional-symbol.s
3

I think its just here to ensure that the __dso_symbol get successfully created by the linker. I'll look into adding something though.

lld/test/wasm/signature-mismatch.ll
79

Its not relevant to the test, and its one less line of boilerplate in the .s file.

llvm/test/MC/WebAssembly/blockaddress.ll
7 ↗(On Diff #265576)

Thats a good question. I was in the process of converting these MC tests when I realized they might also be testing codegen by accident.

Perhaps the best thing to do is to revert all the MC tests here, and consider them separately? (Since the coverage reduction risk only applies to the MC tests not the lld ones).

aardappel added inline comments.May 21 2020, 2:10 PM
lld/test/wasm/Inputs/call-indirect.s
29

.int32 doesn't declare anything, it simply adds static data to the current segment, in this case a relocatable index to foo.

.functype associates signature information with an existing (or new) symbol/label (since signatures are not traditionally part of asm).

sbc100 marked an inline comment as done.May 21 2020, 2:12 PM
sbc100 added inline comments.
lld/test/wasm/Inputs/call-indirect.s
29

The functype is what declares the functions.

.int32 is a data directive. .int32 <symbolname> means insert the 32-bit address of the symbol.

So .int32 foo is effectively taking the address (table offset) of function foo and storing it in static/global data.

The result will be an object file with a relocation of type R_WASM_TABLE_INDEX_I32 pointing to this location in the data section.

sbc100 updated this revision to Diff 267023.May 28 2020, 1:37 PM

More tests ..revert MC tests

sbc100 retitled this revision from [WebAssembly] Convert more lld and MC tests to assembly to [lld][WebAssembly] Convert some lld tests to assembly.May 28 2020, 1:38 PM
sbc100 edited the summary of this revision. (Show Details)
sbc100 edited the summary of this revision. (Show Details)

OK , this is just LLD tests now so should be less controversial. PTAL.

sbc100 edited the summary of this revision. (Show Details)May 28 2020, 1:39 PM
sbc100 edited the summary of this revision. (Show Details)
dschuff accepted this revision.May 28 2020, 3:52 PM
This revision was automatically updated to reflect the committed changes.