This is an archive of the discontinued LLVM Phabricator instance.

[WebAssembly] SIMD extract_lane
ClosedPublic

Authored by tlively on Aug 10 2018, 10:58 PM.

Details

Summary

Implement instruction selection for all versions of the extract_lane
instruction. Use explicit sext/zext to differentiate between
extract_lane_s and extract_lane_u for applicable types, otherwise
default to extract_lane_u.

Patch by Thomas Lively

Diff Detail

Event Timeline

tlively created this revision.Aug 10 2018, 10:58 PM

Looks good

lib/Target/WebAssembly/WebAssemblyInstrSIMD.td
18

Question: How does "#SIZE#" turn it into a number?

22

Is a default value on an argument meaningful when some of subsequent parameters don't have defaults?

34
75

If we don't have these, do they not get selected?

tlively marked an inline comment as done.Aug 13 2018, 6:15 PM
tlively added inline comments.
lib/Target/WebAssembly/WebAssemblyInstrSIMD.td
18

# is like a superpowered strconcat in TableGen. In this case it casts SIZE from an int to a string then appends it to the LaneIdx identifier.

22

Nope, this slipped through a refactoring. Thanks!

34

It doesn't seem to be a drop-in replacement, though. When I replace vector_extract with extractelt I get a TableGen compilation error: Type set is empty for each HW mode: possible type contradiction in the pattern below. The pattern is EXTRACT_LANE_S_I8x16: (sext_inreg:{ *:[i32] } (extractelt:{ *:[] } V128:{ *:[] }:$vec, (imm:{ *:[i32] })<<P:Predicate_LaneIdx16>>:$idx), i8:{ *:[Other] })

75

That's correct. Without these patterns you just get a selection error without an explicit extension.

tlively updated this revision to Diff 160501.Aug 13 2018, 6:19 PM
tlively marked an inline comment as done.
  • Remove useless default arg
aheejin accepted this revision.Aug 14 2018, 10:56 AM
aheejin added inline comments.
lib/Target/WebAssembly/WebAssemblyInstrSIMD.td
34

Hmm, looks like extractelt has [[ https://github.com/llvm-mirror/llvm/blob/e747ac6d9aaaf2d02c3d0a6772285737ea4f08a3/include/llvm/Target/TargetSelectionDAG.td#L241-L243 | SDVecExtract ]] has SDTCisEltOfVec<0, 1> property that's not in vector_extract and it's causing the failure. Looks like, under this property, the result should be the same with the element type of the first argument, which means, i32 = extractelt i8v16 v imm wouldn't work, and I guess that's the reason.

Anyway, vector_extract is still being used widely in LLVM, so I guess that'd be OK.

75

About what we talked in person: I was wondering if scalar loads would have similar rules, but it turned out they do. So nevermind.

This revision is now accepted and ready to land.Aug 14 2018, 10:56 AM
aheejin added inline comments.Aug 14 2018, 11:39 AM
test/CodeGen/WebAssembly/simd.ll
1

Can you remove -wasm-register-codegen-test-mode in this and the other CL? rL339474 was reverted, so we don't have that option yet.

tlively updated this revision to Diff 160654.Aug 14 2018, 11:47 AM
  • Remove useless default arg
  • Fix llc args in test
tlively marked an inline comment as done.Aug 14 2018, 11:52 AM
This revision was automatically updated to reflect the committed changes.