[PATCH] D107502: [WebAssembly] Legalize vector types by widening

Wed Aug 18 14:51:45 PDT 2021

tlively added inline comments.

================
Comment at: llvm/test/CodeGen/WebAssembly/simd-load-store-alignment.ll:297
 ; CHECK-NEXT:    local.get 0
-; CHECK-NEXT:    i16x8.load8x8_u 0:p2align=0
+; CHECK-NEXT:    v128.load64_splat 0:p2align=0
 ; CHECK-NEXT:    # fallthrough-return
----------------
dschuff wrote:
> tlively wrote:
> > aheejin wrote:
> > > It looks this actually changes the result being returned compared to the previously generate code. Is that fine?
> > This is expected because the way we are representing these vector types has changed. It is an ABI break for vector code, but I hope that will be ok in practice. Perhaps it's worth mentioning in the LLVM release notes?
> > 
> > @dschuff, do you have thoughts here?
> > 
> > @srj, will that cause problems for Halide?
> There are sort of 2 issues here.
> One is that our stable ABI is actually a C ABI, and not an LLVM ABI. I forget whether our C ABI (or other C ABIs) actually even defines the convention for vector types, but maybe it should (even if we declare it unstable). It's probably fair to say that we've not promised/stabilized a C vector ABI yet, so if this is a break of some part of the C ABI, I don't know that I'm too worried about it. 
> The other question is whether we want to define and/or stabilize an LLVM ABI, which is of course broader than just C due to the richer (and simultaneously less rich) type system. My sense would be probably not, at least for everything (I don't know of any other platforms that do this, but I haven't thought about it recently). It probably would still be good to inform/consult stakeholders such as Halide (and Rust?) though.
This ABI change does affect the C ABI via the C vector extensions, but our documented C ABI does not say anything about those vector extensions at all, so it sounds like this is not a problem.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107502