[PATCH] D130922: [InstCombine] Add support for stpncpy folding

Martin Sebor via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 27 13:45:20 PDT 2022


msebor added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp:717
   else
-    return nullptr;
+    N = UINT64_MAX;
 
----------------
nikic wrote:
> Can we keep the early return for non-constant size here? As far as I can tell, you don't do anything useful in that case, it just gets discarded later by the `N > 128` check. That's a lot less obvious than immediately rejecting all non-constant sizes here.
We don't want to reject nonconstant sizes because it would prevent the `st{p,r}ncpy(D, "", N)` to `memset(D, '\0', N)` transformation that's done even for any `N`.


================
Comment at: llvm/test/Transforms/InstCombine/stpncpy-1.ll:18
+; (trading space for speed).
+ at str = private constant [4 x i8] c"4\00\00\00"
+ at str.1 = private constant [10 x i8] c"4\00\00\00\00\00\00\00\00\00"
----------------
nikic wrote:
> These are probably intended to have CHECK prefixes? You can use `--check-globals` to generate them.
It took me a bit to understand what you meant here.   I didn't mean to check the contents of these (it's a pre-existing optimization, and I didn't realize I could) but now that I know about `--check-globals` I agree it makes the test tighter.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130922



More information about the llvm-commits mailing list