[PATCH] D130494: [InstCombine] Adjust snprintf folding of constant strings (PR #56598)

[Martin Sebor via Phabricator via llvm-commits]

msebor added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp:2846
     // %% -> % in the future if we cared.
-    if (FormatStr.contains('%'))
-      return nullptr; // we found a format specifier, bail out.
+    if (FormatStr.contains('%') || N > (uint64_t)maxIntN(32))
+      // Bail if the format string contains a formatting directive or
----------------
majnemer wrote:
> Does this do the right thing for targets whose INT_MAX is 16-bit?
This function isn't executed for such targets because it's gated by `TargetLibraryInfoImpl::isValidProtoForLibFunc` which expects `int` to have 32 bits.  This may be an okay assumption to make in this case since (if) most 16-bit targets are bare metal these days, but it's not an unavoidable one (not to mention it's not likely to be intended).  I only last week (in D129915) discovered the relatively recently introduced `getIntSize()` function (added via D99438).  If/when the part of D129915 that lifts the 32-bit assumption is accepted, this function could also be used here and in the rest of the code that makes it.  I'm happy to do that in a follow-up.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130494