[PATCH] D18423: [SimplifyLibCalls] Transform printf("%s", "a") into putchar('a')

[Sanjay Patel via llvm-commits]

spatel added inline comments.

================
Comment at: lib/Transforms/Utils/SimplifyLibCalls.cpp:1843
@@ +1842,3 @@
+  // printf("%s", "a") --> putchar('a')
+  if (FormatStr == "%s" && CI->getNumArgOperands() > 1) {
+    StringRef ChrStr;
----------------
Ok - clang does warn in that case ( warning: data argument not used by format string [-Wformat-extra-args] ), but the extra args are not removed by the front-end, so this looks fine.

================
Comment at: lib/Transforms/Utils/SimplifyLibCalls.cpp:1852
@@ +1851,3 @@
+      return Res;
+    return B.CreateIntCast(Res, CI->getType(), true);
+  }
----------------
Ah, I didn't look far enough ahead to see that guard.
But if we know that CI->use_empty() is always true at this point, then why are we checking it here? If we want to be safe, we could assert that condition, but I don't think that's necessary given that the guard is just a few lines up in the code. Unless I've misunderstood, we should make that change in all of the related printf transforms too.

================
Comment at: test/Transforms/InstCombine/printf-2.ll:13
@@ -10,3 +12,3 @@
 
 declare void @printf(i8*, ...)
 
----------------
Ok - there is an existing negative test to check that the transform doesn't fire when the return value is used in llvm/test/Transforms/InstCombine/printf-1.ll.


http://reviews.llvm.org/D18423