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

[Chris Lattner]

On Nov 20, 2009, at 5:01 PM, Eric Christopher wrote:
> URL: http://llvm.org/viewvc/llvm-project?rev=89523&view=rev
> Log:
> Add more optimizations for object size checking, enable handling of
> object size intrinsic and verify return type is correct. Collect various
> code in one place.

Hi Eric,

> +//===---------------------------------------===//
> +// '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. 

> +  virtual Value *CallOptimizer(Function *Callee, CallInst *CI, IRBuilder<> &B) {
> +    // TODO: We can do more with this, but delaying to here should be no change
> +    // in behavior.
> +    ConstantInt *Const = dyn_cast<ConstantInt>(CI->getOperand(2));
> +
> +    if (!Const) return 0;

This intrinsic *requires* this to be a ConstantInt, right?  If so, change this to be cast<> instead of dyn_cast.  Unlike random function calls, intrinsics have hard requirements that the optimizer can depend on.

> +    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.

> +//===---------------------------------------===//
> +// 'memcpy_chk' Optimizations
> +
> +struct MemCpyChkOpt : public LibCallOptimization {
> +  virtual Value *CallOptimizer(Function *Callee, CallInst *CI, IRBuilder<> &B) {
> +    // These optimizations require TargetData.
> +    if (!TD) return 0;
> +
> +    const FunctionType *FT = Callee->getFunctionType();
> +    if (FT->getNumParams() != 4 || FT->getReturnType() != FT->getParamType(0) ||
> +        !isa<PointerType>(FT->getParamType(0)) ||
> +        !isa<PointerType>(FT->getParamType(1)) ||
> +	!isa<IntegerType>(FT->getParamType(3)) ||
> +	FT->getParamType(2) != TD->getIntPtrType(*Context))
> +      return 0;
> +
> +    ConstantInt *SizeCI = dyn_cast<ConstantInt>(CI->getOperand(4));
> +    if (!SizeCI)
> +      return 0;
> +    if (SizeCI->isAllOnesValue()) {
> +      EmitMemCpy(CI->getOperand(1), CI->getOperand(2), CI->getOperand(3), 1, B);
> +      return CI->getOperand(1);
> +    }

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 :)

Also, please do the check for 'SizeCI' being a constantint before anything else to avoid doing pointless validation.

> +//===---------------------------------------===//
> +// 'memset_chk' Optimizations
> +
> +struct MemSetChkOpt : public LibCallOptimization {
> +  virtual Value *CallOptimizer(Function *Callee, CallInst *CI, IRBuilder<> &B) {
> +    // These optimizations require TargetData.
> +    if (!TD) return 0;
> +
> +    const FunctionType *FT = Callee->getFunctionType();
> +    if (FT->getNumParams() != 4 || FT->getReturnType() != FT->getParamType(0) ||
> +        !isa<PointerType>(FT->getParamType(0)) ||
> +        !isa<IntegerType>(FT->getParamType(1)) ||
> +	!isa<IntegerType>(FT->getParamType(3)) ||
> +        FT->getParamType(2) != TD->getIntPtrType(*Context))
> +      return 0;
> +
> +    ConstantInt *SizeCI = dyn_cast<ConstantInt>(CI->getOperand(4));
> +    if (!SizeCI)
> +      return 0;
> +    if (SizeCI->isAllOnesValue()) {
> +      Value *Val = B.CreateIntCast(CI->getOperand(2), Type::getInt8Ty(*Context),
> +				   false);
> +      EmitMemSet(CI->getOperand(1), Val,  CI->getOperand(3), B);
> +      return CI->getOperand(1);
> +    }

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?

-Chris


> +
> +    return 0;
> +  }
> +};
> +
> +//===---------------------------------------===//
> +// 'memmove_chk' Optimizations
> +
> +struct MemMoveChkOpt : public LibCallOptimization {
> +  virtual Value *CallOptimizer(Function *Callee, CallInst *CI, IRBuilder<> &B) {
> +    // These optimizations require TargetData.
> +    if (!TD) return 0;
> +
> +    const FunctionType *FT = Callee->getFunctionType();
> +    if (FT->getNumParams() != 4 || FT->getReturnType() != FT->getParamType(0) ||
> +        !isa<PointerType>(FT->getParamType(0)) ||
> +        !isa<PointerType>(FT->getParamType(1)) ||
> +	!isa<IntegerType>(FT->getParamType(3)) ||
> +        FT->getParamType(2) != TD->getIntPtrType(*Context))
> +      return 0;
> +
> +    ConstantInt *SizeCI = dyn_cast<ConstantInt>(CI->getOperand(4));
> +    if (!SizeCI)
> +      return 0;
> +    if (SizeCI->isAllOnesValue()) {
> +      EmitMemMove(CI->getOperand(1), CI->getOperand(2), CI->getOperand(3),
> +		  1, B);
> +      return CI->getOperand(1);
> +    }
> +
> +    return 0;
> +  }
> +};
> +
> +//===----------------------------------------------------------------------===//
> // Math Library Optimizations
> //===----------------------------------------------------------------------===//
> 
> @@ -1356,7 +1458,7 @@
>     if (FormatStr == "%c" && CI->getNumOperands() > 2 &&
>         isa<IntegerType>(CI->getOperand(2)->getType())) {
>       Value *Res = EmitPutChar(CI->getOperand(2), B);
> -      
> +
>       if (CI->use_empty()) return CI;
>       return B.CreateIntCast(Res, CI->getType(), true);
>     }
> @@ -1586,7 +1688,10 @@
>     // Formatting and IO Optimizations
>     SPrintFOpt SPrintF; PrintFOpt PrintF;
>     FWriteOpt FWrite; FPutsOpt FPuts; FPrintFOpt FPrintF;
> +
> +    // Object Size Checking
>     SizeOpt ObjectSize;
> +    MemCpyChkOpt MemCpyChk; MemSetChkOpt MemSetChk; MemMoveChkOpt MemMoveChk;
> 
>     bool Modified;  // This is only used by doInitialization.
>   public:
> @@ -1692,9 +1797,13 @@
>   Optimizations["fwrite"] = &FWrite;
>   Optimizations["fputs"] = &FPuts;
>   Optimizations["fprintf"] = &FPrintF;
> -  
> -  // Miscellaneous
> -  Optimizations["llvm.objectsize"] = &ObjectSize;
> +
> +  // Object Size Checking
> +  Optimizations["llvm.objectsize.i32"] = &ObjectSize;
> +  Optimizations["llvm.objectsize.i64"] = &ObjectSize;
> +  Optimizations["__memcpy_chk"] = &MemCpyChk;
> +  Optimizations["__memset_chk"] = &MemSetChk;
> +  Optimizations["__memmove_chk"] = &MemMoveChk;
> }
> 
> 
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits