This is an archive of the discontinued LLVM Phabricator instance.

[WebAssembly] Lower select for vectors
ClosedPublic

Authored by tlively on Oct 24 2018, 2:45 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

tlively created this revision.Oct 24 2018, 2:45 PM
aheejin added inline comments.Oct 30 2018, 11:11 PM
lib/Target/WebAssembly/WebAssemblyInstrSIMD.td
587 ↗(On Diff #170991)

Why do we need this pattern? Can't we also expand SELECT as we did for SELECT_CC and VSELECT?

601 ↗(On Diff #170991)

Nit: Wrap comments to 80 cols

tlively updated this revision to Diff 172060.Oct 31 2018, 5:27 PM
  • Restore return types in names

The commit here looks like replaced with that of D53724.

tlively added inline comments.Oct 31 2018, 5:54 PM
lib/Target/WebAssembly/WebAssemblyInstrSIMD.td
587 ↗(On Diff #170991)

It looks like the expansions bottom out with SELECT. Even when select is set to expand, it still shows up in instruction selection.

tlively updated this revision to Diff 172072.Oct 31 2018, 5:57 PM
  • Wrap comments at col 80. Restore proper revision contents.
tlively marked an inline comment as done.Oct 31 2018, 5:57 PM
tlively updated this revision to Diff 172280.Nov 1 2018, 5:15 PM
  • Wrap comment to 80 chars.
tlively updated this revision to Diff 172708.Nov 5 2018, 8:55 PM
  • Rebase onto SIMD reorganization
tlively edited the summary of this revision. (Show Details)Nov 5 2018, 8:56 PM

I tried to add ISD::SELECT in the expansion list, like

for (auto Op : {ISD::VSELECT, ISD::SELECT_CC, ISD::SELECT}) {
  ...
}

And the code generation apparently works for current test simd-select.ll. I'm not sure if it's optimal though. Could you check again?

tlively updated this revision to Diff 173119.Nov 7 2018, 9:37 PM
  • Use automatic expansion instead of patterns

I tried to add ISD::SELECT in the expansion list, like

for (auto Op : {ISD::VSELECT, ISD::SELECT_CC, ISD::SELECT}) {
  ...
}

And the code generation apparently works for current test simd-select.ll. I'm not sure if it's optimal though. Could you check again?

I'm not sure what I did before, but you're totally right. Letting the expander take care of selects yields the same code in most places and actually allows some DAGCombine optimizations that weren't possible before.

tlively updated this revision to Diff 173120.Nov 7 2018, 9:38 PM
  • Update comment
aheejin accepted this revision.Nov 8 2018, 2:46 PM
This revision is now accepted and ready to land.Nov 8 2018, 2:46 PM
This revision was automatically updated to reflect the committed changes.
llvm/trunk/test/CodeGen/WebAssembly/simd-select.ll