This is an archive of the discontinued LLVM Phabricator instance.

[WebAssembly] Update SIMD binary arithmetic
ClosedPublic

Authored by tlively on Aug 3 2018, 7:17 PM.

Details

Summary

Add missing SIMD types (v2i64 and v2f64) and binary ops. Also adds
tablegen support for automatically prepending prefix byte to SIMD
opcodes. Replaces individual vector ExprTypes with a single V128 type.

Diff Detail

Event Timeline

tlively created this revision.Aug 3 2018, 7:17 PM

Looks OK with some formatting nits

lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp
261

Can you clang-format this?

lib/Target/WebAssembly/WebAssemblyInstrFormats.td
63

Nit: space between simdop and =

lib/Target/WebAssembly/WebAssemblyRegisterInfo.td
66

Wrap to 80 cols?

dschuff added inline comments.Aug 6 2018, 2:37 PM
lib/Target/WebAssembly/InstPrinter/WebAssemblyInstPrinter.cpp
214

Replacing the ExprType is not related to the rest of this CL, so it should probably be split into a separate CL.

test/CodeGen/WebAssembly/simd-arith.ll
150

missing div?

tlively updated this revision to Diff 159434.Aug 6 2018, 4:55 PM
tlively marked 5 inline comments as done.
  • Formatting in .td files

Landed rL339082 to address comments.

lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp
261
test/CodeGen/WebAssembly/simd-arith.ll
150

SIMD integer div ops are not in the spec

aheejin accepted this revision.Aug 6 2018, 5:05 PM
This revision is now accepted and ready to land.Aug 6 2018, 5:05 PM
tlively updated this revision to Diff 159584.Aug 7 2018, 1:17 PM
  • Formatting in .td files
  • Remove i64x2.mul since it is not in spec
tlively updated this revision to Diff 159588.Aug 7 2018, 1:24 PM
  • Revert "Remove i64x2.mul since it is not in spec." It is unclear this
dschuff accepted this revision.Aug 7 2018, 2:20 PM

Keeping MUL is fine for now. IIRC there was some discussion in the last CG in-person meeting about removing or changing it (e.g. probably not super-useful especially for small types due to the likelihood of overflow). Probably in cases where the spec is still in flux it makes sense for LLVM to match what V8 implements now, so we can test things end-to-end. Then when we settle on things we can just change LLVM (or do LLVM first to see whether a proposal is feasible).

This revision was automatically updated to reflect the committed changes.