[PATCH] D73000: [InstCombine] Optimize strchr(cststr, C)

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp:932
   // this memchr call into a simple bit field test. Of course this only works
   // when the return value is only checked against null.
   //
----------------
Unrelated: Even if it is not only checked against null we can do this and add a nonnull result to the string pointer, right?


================
Comment at: llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp:948
     // target.
-    // FIXME: On a 64 bit architecture this prevents us from using the
-    // interesting range of alpha ascii chars. We could do better by emitting
-    // two bitfields or shifting the range by 64 if no lower chars are used.
-    if (!DL.fitsInLegalInteger(Max + 1))
-      return nullptr;
+    if (!DL.fitsInLegalInteger(Max + 1)) {
+      // Build chain of OR comparisons
----------------
hans wrote:
> xbolva00 wrote:
> > joerg wrote:
> > > The switch table logic is fine with lowering it to a multiple tests, isn't it?
> > Hmm.. building switch directly is for some cases* better choice** than ORs, but I am wondering if this is best solution here -  Maybe @hans could give us some recommendations here?
> > 
> > * https://godbolt.org/z/BuR6St (switch is faster than ors).
> > ** Ideally, we should turn chain of ORs to switch too.
> > 
> > Ideas?
> > 
> > 
> I think we (SimplifyCFG) does turn a chain of ORs into a switch, but only logical or, not bitwise. Maybe emitting  a chain of logical or would be good here?
We should add a test with instcombing and simplifyCFG to make sure this works.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73000