[PATCH] D123818: [InstCombine] Fold strnlen calls in equality to zero.

[Martin Sebor via Phabricator via llvm-commits]

msebor added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp:645
+    //   strnlen(x, N) == 0 --> *x == 0
+    return B.CreateZExt(B.CreateLoad(CharTy, Src, "strlenfirst"),
+                        CI->getType());
----------------
nikic wrote:
> msebor wrote:
> > xbolva00 wrote:
> > > maybe you can drop “strlenfirst” here.. 
> > Because the function also handles `strnlen` besides `strlen`?  I can do that but since other loads in the file are annotated I've changed it to `"char0"`.  Let me know if I misunderstood.
> Looks like this rename did not happen (though I don't really care either way).
It did, I just haven't posted the updated patch yet.  Now that it's been accepted I'll go ahead and commit it with this change.


================
Comment at: llvm/test/Transforms/InstCombine/strnlen-5.ll:42
 
 ; Fold strnlen(ax, 1) == 0 to *ax == 0.
 
----------------
xbolva00 wrote:
> msebor wrote:
> > xbolva00 wrote:
> > > What about “strnlen(…) < 1” ? Do we have such test?
> > I've added a couple.  I expected `isOnlyUsedInZeroEqualityComparison()` to return true for such expressions but it only handles straight equality.  The case is folded by the subsequent code path but the emitted code is slightly different.  It seems it might be worth looking into enhancing the function to handle both `< 1` and `== 0` with unsigned operands (as well as the converse).
> No need as < 1 is always canonicalized to == 0.
Yes, ultimately it is, but it seems to me that it would be helpful to follow the same code path and make the same folding decisions earlier on, in the libcall handler.  I don't have a use case for it yet where it would make a difference (the function isn't widely used) but if one comes up I'll revisit it then,


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123818