This is an archive of the discontinued LLVM Phabricator instance.

[WebAssembly][NFC] Reorganize SIMD instructions
ClosedPublic

Authored by tlively on Aug 22 2018, 11:16 AM.

Details

Summary

This CL moves SIMD-specific instruction formats out of
WebAssemblyInstrFormats.td to improve code locality. It also
reorganizes WebAssemblyInstrSIMD.td to put all of the instruction
definitions together, making it easier to see which instructions have
been implemented already. Depends on D51082.

Diff Detail

Repository
rL LLVM

Event Timeline

tlively created this revision.Aug 22 2018, 11:16 AM
tlively updated this revision to Diff 162046.Aug 22 2018, 1:38 PM
  • Remove extra spaces
aheejin added inline comments.Aug 22 2018, 4:13 PM
lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
128 ↗(On Diff #162046)

arc tip: When you submit a CL that depends on another one, run arc diff with a relative commit, like, arc diff HEAD^2 or arc diff [hash]. In that way the diff of the prev revision that this CL depends on would not show up in this CL again.

tlively updated this revision to Diff 162095.Aug 22 2018, 4:16 PM
  • Remove extra spaces
tlively added inline comments.Aug 22 2018, 4:17 PM
lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
128 ↗(On Diff #162046)

I thought I had done that, but maybe I messed up. Anyway, rebased and updated now.

aheejin added inline comments.Aug 22 2018, 4:23 PM
lib/Target/WebAssembly/WebAssemblyInstrSIMD.td
91 ↗(On Diff #162095)

It seems weird that we have SIMD instruction formats in its own file but other formats like integer/fp arithmetic still in WebAssemblyInstrFormats.td. Can you make a separate CL that pulls all those patterns, including SIMD ones, out of that file and puts into more relevant files, like, WebAssemblyInstrInteger.td for integer arithmetic and such?

tlively updated this revision to Diff 162113.Aug 22 2018, 5:21 PM
  • rebase onto change from D51143

Well the diff is definitely messed up now, but I'm sure it will be fixed once D51143 lands.

Phabricator does not care about which commits have landed or not. It only takes diff of the two commits (your branch tip vs the commit you specified; if missing master I guess), and because you don't commit CLs using Phabricator, they two are actually independent. So, fixing this diff does not require your dependent CLs to land, and also, even if the diff is messed up here, it does not mean your commits will be messed up. Anyway, it's good to fix diff for readability. :)

tlively updated this revision to Diff 162117.Aug 22 2018, 5:33 PM

Diff against the correct commit

aheejin accepted this revision.Aug 22 2018, 5:35 PM
This revision is now accepted and ready to land.Aug 22 2018, 5:35 PM
This revision was automatically updated to reflect the committed changes.