[PATCH] D56457: [WebAssembly][FastISel] Do not assume naive CmpInst lowering

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: lib/Target/WebAssembly/WebAssemblyFastISel.cpp:451
+    // not a DAG ISel fallback. TODO: Recursively examine selects, phis, and,
+    // or, xor, constants.
     if (From == MVT::i1 && V != nullptr) {
----------------
tlively wrote:
> tlively wrote:
> > aheejin wrote:
> > > Maybe I'm lacking the context. How does this code check if this is lowered by FastISel of DAG ISel fallback?
> > > And real nit: Can we place the TODO in the next line? 
> > I do not know of a way to tell whether or how something has already been lowered, so I just removed the optimization. I suppose to be safe I should remove the Argument optimization as well, since I certainly can't prove that it's correct. This change makes us generate worse code in lots of places, but since it is FastISel perhaps it doesn't matter so much?
> I just removed the TODO. It is irrelevant anyway if we don't figure out how to tell if something has been lowered by FastISel.
FastISel selects instructions in reverse order, so at this point the icmp hasn't been selected yet and there's no way to tell -- it would only be possible to replicate the checks FastISel does and ensure they'll all be true. That seems somewhat fragile though.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D56457