[PATCH] D101684: [WebAssembly] Add end-to-end codegen tests for wasm_simd128.h

David Blaikie via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sun May 2 14:39:38 PDT 2021


dblaikie added a comment.

In D101684#2732408 <https://reviews.llvm.org/D101684#2732408>, @tlively wrote:

> In D101684#2732395 <https://reviews.llvm.org/D101684#2732395>, @dblaikie wrote:
>
>> In D101684#2732366 <https://reviews.llvm.org/D101684#2732366>, @tlively wrote:
>>
>>> In order to get the benefit of this end-to-end test from split tests like that, the LLVM test would have to be automatically generated from the clang test.
>>
>> Why is that? We don't do that for other test surface area between clang and LLVM.
>
> The question this test answers is "Do the intrinsic functions generate the proper WebAssembly instructions?"  (Notably, the test reveals that in multiple cases, they don't). If we had separate C->IR and and IR->Wasm tests, they would be able to answer this question only if we were sure that the output of the C test matched the source of the IR test, and generating the IR test from the C test would be the best way to ensure that.
>
> I understand your point that clang tests typically do not try to answer this kind of question, but this is an important question to be able to answer for the folks working on WebAssembly SIMD.

Is it fundamentally differently/more important than all the other questions like ABI compatibility? In what way?

> So the options I see are:
>
> 1. Have this abnormal end-to-end test in clang.
> 2. Autogenerate an IR test from the C test so the composition of tests tells us what we want to know.
> 3. Host the test in some other repository.
>
> Among those, the first is both the easiest to maintain and the most useful.

My main objection is to the testing itself compared to the rest of the clang/llvm test philosophy - mostly in the hopes that splitting such testing, the same as nearly all other testing, would be sufficient here. If not, it might be nice to understand what kinds of test/properties make this suitable here despite it generally not being suitable for what seem like similar issues elsewhere in the compiler.

Also, other platforms seem to be OK with this sort of split testing - there's lots of testing of intrinsics (mostly in clang/test/CodeGen, rather than clang/test/Headers, by the looks of it) which, at a cursory glance, seems to generally use emit-llvm+FileCheck, not going all the way to assembly. I don't know I've seen much discussion that this has been a problematic gap in testing for LLVM targets so far.

(all that said, if it's really needed, there's something that makes it fundamentally different from what LLVM's done historically here, or evidence that's been problematic/costly in terms of allowing regressions, I don't mind them being in the clang test directory - though there's also the currently-being-refactored debuginfo-tests directory which will also be for higher level more end-to-end tests and these tests might be suitable there... though again, be good to understand why the current testing for other targets has been inadequate or what makes WebAssembly different here)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101684/new/

https://reviews.llvm.org/D101684



More information about the cfe-commits mailing list