This is an archive of the discontinued LLVM Phabricator instance.

[WebAssembly] General vector shift lowering
ClosedPublic

Authored by tlively on Oct 23 2018, 5:24 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

tlively created this revision.Oct 23 2018, 5:24 PM
tlively updated this revision to Diff 170801.Oct 23 2018, 6:14 PM
  • Remove stray debug statement
aheejin added inline comments.Oct 31 2018, 6:14 PM
lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
1027 ↗(On Diff #170801)

We did setOperationAction to Custom only for vector types, so is there a case this if can be satisfied? How about changing this to an assertion this is a vector type?

1034 ↗(On Diff #170801)

On an unrelated note, maybe we can use this function again to conditionally expand extract_lane?

1036 ↗(On Diff #170801)

Do we have test cases for non-const splat?

1055 ↗(On Diff #170801)

This isn't added in this CL, but we probably don't need this line after llvm_unreachable

tlively marked 2 inline comments as done.Nov 1 2018, 10:04 AM
tlively added inline comments.
lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
1034 ↗(On Diff #170801)

UnrollVectorOp uses extract_lane to apply the op to each lane individually, so it can't be used to lower extract_lane itself. But D53964 has a similarly simple solution.

1036 ↗(On Diff #170801)

Yes, see @splat_XXX functions in simd.ll.

tlively updated this revision to Diff 172190.Nov 1 2018, 11:35 AM
  • Address comments
aheejin added inline comments.Nov 1 2018, 2:43 PM
lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
1072 ↗(On Diff #172190)

This comment is confusing, because it sounds like i64x2 non-const splats are not handled by patterns. But by looking at the code, it looks like

  • non-i64x2 non-const splat: handled by pattern
  • i64x2 non-const splat: handled by pattern
  • non-i64x2 const splat: handled by pattern
  • i64x2 const splat: custom lowering

So the only case we use the custom logic below is i64x2 const splat, right? I think the comment says otherwise.

tlively updated this revision to Diff 172283.Nov 1 2018, 5:30 PM
tlively marked an inline comment as done.
  • Fix confusing comment
aheejin accepted this revision.Nov 1 2018, 5:32 PM
This revision is now accepted and ready to land.Nov 1 2018, 5:32 PM
This revision was automatically updated to reflect the committed changes.