[PATCH] D122686: [InstCombine] Fold calls to strnlen (PR46455)

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp:747
+      return B.CreateBinaryIntrinsic(Intrinsic::umin, Res, Bound);
+    };
   }
----------------
msebor wrote:
> nikic wrote:
> > This is redundant: You already computed the bounded string lengths for both branches above, so there should be no need to bound the result here again.
> I agree the `umin` is not necessary.  At the same time, removing it makes the code look like it's missing.  It should get folded away, but I don't have a strong preference one way or the other.
The alternative here would be to first always calculate the unbounded string length and then do the umin once at the end. Actually, I think that would be cleaner than the current code structure, because most of the logic would be independent of the bound, and we'd only have to insert the umin at one place at the end, rather than in each branch of the string length computation.


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

https://reviews.llvm.org/D122686