[llvm-commits] [llvm] r89523 - /llvm/trunk/lib/Transforms/Scalar/SimplifyLibCalls.cpp

[Chris Lattner]

On Dec 4, 2009, at 1:03 AM, Eric Christopher wrote:

>> 
>> 
>>> +//===---------------------------------------===//
>>> +// 'object size'
>>> +namespace {
>>> +struct SizeOpt : public LibCallOptimization {
>> 
>> This code (if it is kept) should be moved to instcombine, and generalized to work on more than just i32/i64.  IT should work on any llvm.objectsize.ixxx intrinsic call. 
> 
> Well, there are only two we generate at the moment,

Sure, that is true of the current frontend that generates this, but the optimizer should be generic.

> but it could be adapted to any since we're just returning a length.  I'm not sure about instcombine itself, but this is likely not the place to put it for sure :)

This code should go in instcombine in the InstCombiner::visitCallInst method.

>> 
>>> +    const Type *Ty = Callee->getFunctionType()->getReturnType();
>>> +
>>> +    if (Const->getZExtValue() < 2)
>>> +      return Constant::getAllOnesValue(Ty);
>>> +    else
>>> +      return ConstantInt::get(Ty, 0);
>> 
>> I don't really get this.  Why is libcalloptimizer turning these into "I don't know"?  Shouldn't codegen (e.g. codegenprepare) do this?  This seems really wrong.
> 
> It's doing this now so that these other optimizations below can replace the checking calls with normal calls. It also matches what llvm-gcc did before.  That said, we are producing less checks than we should - I went for keeping existing behavior over trying to get it all right at once.

This should happen in codegenprepare?  libcallsimplify effectively runs at an arbitrary time in the optimizer sequence.  We want the optimizer to be able to prove away these calls (which is why having instcombine do the proving is useful) whenever it can, and then lower them to 'unknown' (and simplify the users) right before codegen.

>> This should also handle the case when getOperand(3) and getOperand(4) are both constant ints and op3 <= op4?  If we know that op3 > op4, I'm fine with leaving it unoptimized :)
>> 
> 
> Good question. I wasn't sure if this pass was iterative? I.e. if I change it to memcpy can the rest of the pass infrastructure deal with it? Of course, I could just look and figure it out - I just hadn't gotten that far :)

Yes, it is iterative.

>> Can this code be factored better?  Maybe introduce a new intermediate class that all of MemCpyChkOpt/MemSetChkOpt/MemMoveChkOpt inherit from that then inherits from LibCallOptimization?
> 
> Most assuredly. It was bugging me too :)

Ok, please do, thanks!

-Chris