This is an archive of the discontinued LLVM Phabricator instance.

[WebAssembly] Regenerate SIMD Arithmetic tests
AbandonedPublic

Authored by RKSimon on Aug 4 2019, 10:18 AM.

Details

Summary

This is a massive change so wanted to get feedback from the webassembly people if its an acceptable approach or not.

An upcoming SimplifyDemandedBits patch of mine changes the wasm vector shift codegen a huge amount and I was hoping if moving this file to autogeneration is the way to go to make future updates easier.

I've used the update_llc_test_checks script to regenerate the test file, and have added additional check prefixes to reduce duplication.

Because of PR42882, there still needs to be some minor manual tweaking to remove the %BB tags from the checks, if we can remove -asm-verbose=false from the llc command lines then the regeneration would be completely automated.

Diff Detail

Repository
rL LLVM

Event Timeline

RKSimon created this revision.Aug 4 2019, 10:18 AM

I have mixed feelings about this. One one hand I understand that this file is huge and cross-cutting changes could cause a lot of manual updating work, but on the other hand each individual test was very readable and succinct. The auto-generated version would certainly be less work to maintain, but its enormous amount of irrelevant information and repetition is unfortunate. @aheejin, what do you think about this?

quantum resigned from this revision.Aug 5 2019, 1:12 PM

I have never touched the SIMD code and as a result, am not qualified to review this.

I have mixed feelings about this. One one hand I understand that this file is huge and cross-cutting changes could cause a lot of manual updating work, but on the other hand each individual test was very readable and succinct. The auto-generated version would certainly be less work to maintain, but its enormous amount of irrelevant information and repetition is unfortunate. @aheejin, what do you think about this?

I have the same concerns. While I understand the convenience of using the auto generation script, the file is now far less readable and contains a lot more contents, and also all different test prefixes, some of which needed only for a few tests, are generated for every function. With the file being huge, it would also be hard to see if subsequent updates to the test are correct.

RKSimon abandoned this revision.Aug 6 2019, 1:42 AM

This is what I figured - I'll abandon this, but will probably need you guys to massage the checks for the upcoming SimplifyDemandedBits patch.

Sounds good; I'm happy to help out.