[llvm-commits] [PATCH] Optimize printf -> iprintf if there are no floating point arguments

[Richard Osborne]

On 02/03/11 01:15, Chris Lattner wrote:
> On Mar 1, 2011, at 9:05 AM, Richard Osborne wrote:
>
>> This patch adds an optimization to SimplifyLibCalls which transforms calls to printf into iprintf calls. iprintf is a restricted version of printf provided by newlib without floating-point formatting (see http://lists.cs.uiuc.edu/pipermail/llvmdev/2011-February/037832.html).
>>
>> This patch uses TargetLibraryInfo to determine whether iprintf is available. Currently it is only marked as available on the XCore target.
>>
>> I intend to add similar optimizations for sprintf / fprintf once this patch is accepted. Please let me know if this is OK to commit.
> In addition to Frits' comment about the testcase, the patch would be simpler if you changed this:
>
> +    // printf(format, ...) ->  iprintf(format, ...) if no floating point
> +    // arguments.
> +    if (TLI->has(LibFunc::iprintf)&&  !CallHasFloatingPointArgument(CI)) {
> +      return EmitIPrintF(CI->op_begin(), CI->op_end(), B, TD);
> +    }
>
> Instead of emitting a new IPrintF call, you can just change the callee of the existing call to be iprintf instead of printf.  This would make the code look something like this:
>
>      // printf(format, ...) ->  iprintf(format, ...) if no floating point
>      // arguments.
>      if (TLI->has(LibFunc::iprintf)&&  !CallHasFloatingPointArgument(CI)) {
>        Constant *IPrintFFn = ...
>        CI->setOperand(0, IPrintFFn);
>        return CI;
>      }
>
> This allows you to remove the EmitIPrintF implementation and should simplify the fprintf case that's coming next.
>
> Thanks for implementing this Richard!
>
> -Chris
If you return the existing call then SimplifyLibCalls tries to erase it, 
see the following comment:

   /// CallOptimizer - This pure virtual method is implemented by base 
classes to
   /// do various optimizations.  If this returns null then no 
transformation was
   /// performed.  If it returns CI, then it transformed the call and CI 
is to be
   /// deleted.  If it returns something else, replace CI with the new 
value and
   /// delete CI.

I've change the code to clone the call and return the clone after 
changing the callee. This means EmitIPrintF is no longer needed.

I've also fixed the issue in the testcase spotted by Frits.

Committed in r126935

Thanks for the review comments.

-- 
Richard Osborne | XMOS
http://www.xmos.com