[LLVMdev] Implementing platform specific library call simplification

[John Criswell]

On 2/24/2011 1:21 AM, Chris Lattner wrote:
> On Feb 13, 2011, at 12:24 AM, Chris Lattner wrote:
>
>> On Feb 2, 2011, at 10:11 AM, Richard Osborne wrote:
>>> The newlib C library provides iprintf(), a restricted version of printf
>>> without support for floating-point formatting. I'd like to add an
>>> optimization which turns calls to printf() into calls to iprintf() if
>>> the format string has no floating point specifiers.
>> Cool, ok.  I can see how this would be very useful for a variety of embedded systems.
>>
>>> At the moment I've got this working locally by adding code to the
>>> simplify-libcalls pass. However this will break on targets where
>>> iprintf() isn't available. Is there a sensible place to add information
>>> about which library functions are available for a particular target?
>> I'd suggest adding a bool argument (HasIPrintf) to the createSimplifyLibCallsPass function and the pass constructor.  Then clang (or whatever is setting up the pass manager) can query the appropriate target info to pass down this flag.

Sorry to jump into something midstream, but I just happened to read the 
above paragraph and thought I should comment.  Adding a boolean argument 
to a pass's constructor method is one of the things I have learned not 
to do.  If I ever give a "How Not to Write an LLVM Pass" talk, this will 
be one of the things in the list.

The problem with boolean default arguments in pass constructors is that 
they make a pass more difficult to use within the opt and bugpoint 
tools.  Inevitably you'll find some bug that only occurs when the flag 
is set to a non-default value, you'll fire up bugpoint, pass in the 
arguments printed by your tool's -debug-pass=Arguments option, and then 
wonder why the bug doesn't trigger.  Eventually you find out that it's 
due to the boolean flag you put into the constructor not being set the 
way you want.  You then have to manually change the constructor in the 
source code, recompile, run bugpoint, and then remember to change it 
back before committing your fix.

Perhaps you can escape this problem because you're working on a pass 
that is hard-coded into opt and bugpoint, but it'll only work if opt and 
bugpoint and whatever special tool that this pass is incorporated into 
all set that boolean in the same way under the same conditions.  If 
someone uses it in a tool that sets it using some non-standard criteria, 
then you run into this problem.

We went through this with Automatic Pool Allocation and SAFECode: it was 
one of several things that made bugpoint inconvenient or impossible to use.

I have not actually fixed this problem in Poolalloc/SAFECode yet, but 
I've got the following ideas to try:

1) Create one version of your pass that sets the boolean flag to true 
and another version of your pass that sets the boolean flag to false.  
You can probably use C++ templates or inheritance to do this.  Register 
each pass as a separate pass and have your tool pick which pass to run 
based upon whatever conditions you want.

For example, the Poolalloc pass has to use either EQTDDataStructures or 
EQBUDataStructures pass depending on whether it is used for SAFECode.  
Right now it picks one based on a boolean constructor.  Instead, I would 
have the Poolalloc pass be a template that takes the name of the pass to 
use as input and create two passes:Poolalloc<EQTDDataStructures> and 
Poolalloc<EQBUDataStructures>.  SAFECode would use 
Poolalloc<EQTDDataStructures>.

2) Create an analysis group that tells your pass what to do (i.e., your 
pass calls a method on the analysis group to get the boolean flag).  
Create two immutable passes belonging to the group and return different 
values for the boolean flag.

Either of these techniques will ensure that any PassManager based tool 
can print out a string with -debug-pass=Arguments that can be fed into 
opt or bugpoint to reproduce the same behavior.

My 4 cents.

-- John T.

>> Longer term, I'd like to introduce a simple interface (via TargetRegistry) for exposing target libcall info.  This would be useful to eliminate the HAVE_FLOORF etc #defines in simplifylibcalls, which break cross compilation.
> Hi Richard,
>
> Just to close the loop on this, I went ahead and added a new llvm/Target/TargetLibraryInfo.h and added it to simplifylibcalls.  TargetLibraryInfo doesn't handle all the libcalls that SimplifyLibCalls handles yet, but it should provide a starting point for iprintf: just add an iprintf enum to TargetLibraryInfo and make your transformation predicated on TLI->has(LibFunc::iprintf).
>
> It would be great to see this go into mainline,
>
> -Chris
>
>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev