[PATCH] D51459: [WebAssembly][WIP] Vector conversions

Wouter van Oortmerssen via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 30 12:09:07 PDT 2018


aardappel added inline comments.


================
Comment at: lib/Target/WebAssembly/WebAssemblyRegStackify.cpp:569
+  if (WasmDisableExplicitLocals)
+    return Def;
+
----------------
tlively wrote:
> aardappel wrote:
> > aheejin wrote:
> > > tlively wrote:
> > > > @aardappel This solution for the problem described in the comment fixes my own test, but causes crashes in the stackification tests. I assume this function is doing other work besides creating a TEE that cannot be skipped. Could you suggest a more robust way to fix this problem?
> > > (We talked in person and I'm gonna summarize it here)
> > > Giving those instructions fake opcodes (like 0) can be one solution too.
> > Yes, we'd really want to avoid checks like this, as disabling the explicit locals is something pretty hacky we only do to keep current tests happy.
> > 
> > I think the easiest solution is to convert the affected test to work with explicit locals?
> Would it be useful to convert the tests to work with explicit locals or is there some other benefit to disabling them?
Yes, that is generally useful, we'd like to move to a world where there is as little use of this flag (and --wasm-keep-registers) as possible, with the exception maybe of test that specifically are about the handling of locals and registers in IR.
The presence of these flags complicates matters, e.g. we are still not able to assume in MC that all instructions are the stack versions.


Repository:
  rL LLVM

https://reviews.llvm.org/D51459





More information about the llvm-commits mailing list