This is an archive of the discontinued LLVM Phabricator instance.

[NFC][WebAssembly] Autogenerate test expectations for tailcall.ll
ClosedPublic

Authored by tlively on Mar 21 2023, 2:43 PM.

Details

Summary

A follow-on commit will add tests to this file and using the
update_llc_test_checks script will make that easier.

Diff Detail

Event Timeline

tlively created this revision.Mar 21 2023, 2:43 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 21 2023, 2:43 PM
Herald added subscribers: pmatos, asb, wingo and 4 others. · View Herald Transcript
tlively requested review of this revision.Mar 21 2023, 2:43 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 21 2023, 2:43 PM
sbc100 added inline comments.Mar 21 2023, 2:45 PM
llvm/test/CodeGen/WebAssembly/tailcall.ll
105

Is there some way to avoid emitting these comments and keep the expectations smaller?

tlively added inline comments.Mar 21 2023, 2:51 PM
llvm/test/CodeGen/WebAssembly/tailcall.ll
105

Not without some extra work. The update script currently only recognizes the fully verbose output for the Wasm backend.

dschuff accepted this revision.Mar 21 2023, 2:59 PM

LGTM I guess. TBH I'm still a little on the fence about whether it's better to have fully auto-generated test expectations which are easy to update but check all the output, or more precisely targeted expectations which are more self-documenting (since they don't include output not relevant to the test at hand) and are less likely to require updating for unrelated changes...

This revision is now accepted and ready to land.Mar 21 2023, 2:59 PM
aheejin accepted this revision.Mar 21 2023, 3:16 PM

I'm on the fence too. Are we going to (eventually) change all tests expectations? I think there are tests that can really benefit from the brevity (especially tests added with bugfixes, with which we only want to check a specific few lines of the output). But this test may not be one of them.

I'm on the fence too. Are we going to (eventually) change all tests expectations? I think there are tests that can really benefit from the brevity (especially tests added with bugfixes, with which we only want to check a specific few lines of the output). But this test may not be one of them.

That's a good point; these tests are quite short and very targeted IR (as opposed to tests that require complex situations to manifest bugs).

Yeah, I agree that many tests are better with smaller, manually written expectations. For these particular tests, the generated expectations aren't that long and are easy to scan for the expected instructions. Using the full output also helps show that the arguments, epilogues, and other surrounding code looks good as well.