[PATCH] D108496: [WebAssembly] Lower v2f32 to v2f64 extending loads with promote_low

[Heejin Ahn via Phabricator via llvm-commits]

aheejin accepted this revision.
aheejin added a comment.
This revision is now accepted and ready to land.

Thanks for the explanation! Then after this CL, does `promote_low` part for LowerConvertLow <https://github.com/llvm/llvm-project/blob/4387975170112ad51a6cb57b5b77ebd72cc73c57/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp#L1781-L1844> become redundant? Hmm, maybe not completely, because this has a capability of shuffling <https://github.com/llvm/llvm-project/blob/4387975170112ad51a6cb57b5b77ebd72cc73c57/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp#L1837-L1842>? (By the way we don't seem to have a test of shuffled `promote_low` case.. simd-conversions.ll <https://github.com/llvm/llvm-project/blob/main/llvm/test/CodeGen/WebAssembly/simd-conversions.ll> only contains integer vector to v2f64 tests. It would be good to have one for float vectors to v2f64 too) But given that `LowerConvertLow` function also takes care of integer to float conversions, we cannot remove that function anyway though. (Unless we do a similar optimization for integer vectors)



================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyInstrSIMD.td:1283
+def extloadv2f32 : PatFrag<(ops node:$ptr), (unindexedload node:$ptr)> {
+  let IsLoad = true;
+  let IsAnyExtLoad = true;
----------------
tlively wrote:
> aheejin wrote:
> > Is this necessary? `unindexedload` already sets `IsLoad` to `true`.
> I believe so. We are inheriting from `PatFrag`, not from `unindexedload` directly, and I think the whole new fragment has to be marked as a load. Looking at the definitions of e.g. extloadf32, though, we could use `extload` instead of `unindexedload` and get rid of the `IsAnyExtLoad = true` line.
That's nice. By the way `extload` also sets `IsLoad` to `true`, so this line will still be unnecessary.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108496/new/

https://reviews.llvm.org/D108496