[llvm-commits] [llvm] r122959 - in /llvm/trunk: lib/Target/README.txt lib/Transforms/InstCombine/InstCombineCalls.cpp test/Transforms/InstCombine/objsize.ll

[Bob Wilson]

On Jan 6, 2011, at 5:11 AM, Benjamin Kramer wrote:

> Author: d0k
> Date: Thu Jan  6 07:11:05 2011
> New Revision: 122959
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=122959&view=rev
> Log:
> InstCombine: If we call llvm.objectsize on a malloc call we can replace it with the size passed to malloc.
> 
> Modified:
>    llvm/trunk/lib/Target/README.txt
>    llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp
>    llvm/trunk/test/Transforms/InstCombine/objsize.ll

This caused a significant performance regression in the MultiSource/Benchmarks/Prolangs-C/cdecl test:
http://llvm.org/perf/db_default/simple/nts/69/

The compile-time regression is just a symptom of having larger code.  The ds() function in cdecl.c is:

char *ds(s)
char *s;
{
    register char *p = malloc((unsigned)(strlen(s)+1));

    if (p)
	(void) strcpy(p,s);
    else
	{
	(void) fprintf (stderr, "%s: malloc() failed!\n", progname);
	exit(1);
	}
    return p;
}

Before this change, codegenprepare folded the strcpy_chk to strcpy because it treated the malloc size as unknown.  Now, it knows the malloc size but not the source size, so it's keeping the strcpy_chk.  The ds() function is inlined to lots of places, so the effect is greatly magnified.  In this case, it ought to be possible to recognize that the source is the same size as the destination and have instcombine fold to strcpy.  Could you look into doing that, or at least add a comment to the README file about it?

> 
> Modified: llvm/trunk/lib/Target/README.txt
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/README.txt?rev=122959&r1=122958&r2=122959&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/README.txt (original)
> +++ llvm/trunk/lib/Target/README.txt Thu Jan  6 07:11:05 2011
> @@ -2020,29 +2020,6 @@
> 
> //===---------------------------------------------------------------------===//
> 
> -This code can be seen in viterbi:
> -
> -  %64 = call noalias i8* @malloc(i64 %62) nounwind
> -...
> -  %67 = call i64 @llvm.objectsize.i64(i8* %64, i1 false) nounwind
> -  %68 = call i8* @__memset_chk(i8* %64, i32 0, i64 %62, i64 %67) nounwind
> -
> -llvm.objectsize.i64 should be taught about malloc/calloc, allowing it to
> -fold to %62.  This is a security win (overflows of malloc will get caught)
> -and also a performance win by exposing more memsets to the optimizer.
> -
> -This occurs several times in viterbi.
> -
> -Stuff like this occurs in drystone:
> -
> -  %call5 = call i8* @malloc(i32 48) optsize
> -  %5 = getelementptr inbounds i8* %call5, i32 16
> -  %6 = call i32 @llvm.objectsize.i32(i8* %5, i1 false)
> -
> -We should be able to constant fold that.
> -
> -//===---------------------------------------------------------------------===//
> -
> This code (from Benchmarks/Dhrystone/dry.c):
> 
> define i32 @Func1(i32, i32) nounwind readnone optsize ssp {
> 
> Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp?rev=122959&r1=122958&r2=122959&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp (original)
> +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp Thu Jan  6 07:11:05 2011
> @@ -298,12 +298,16 @@
>         }
>       }
>     } else if (CallInst *MI = extractMallocCall(Op1)) {
> -      // Get alloca size.
> +      // Get allocation size.
>       const Type* MallocType = getMallocAllocatedType(MI);
>       if (MallocType && MallocType->isSized())
>         if (Value *NElems = getMallocArraySize(MI, TD, true))
>           if (ConstantInt *NElements = dyn_cast<ConstantInt>(NElems))
>             Size = NElements->getZExtValue() * TD->getTypeAllocSize(MallocType);
> +
> +      // If there is no offset we can just return the size passed to malloc.
> +      if (Offset == 0)
> +        return ReplaceInstUsesWith(CI, MI->getArgOperand(0));
>     }
> 
>     // Do not return "I don't know" here. Later optimization passes could
> 
> Modified: llvm/trunk/test/Transforms/InstCombine/objsize.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/objsize.ll?rev=122959&r1=122958&r2=122959&view=diff
> ==============================================================================
> --- llvm/trunk/test/Transforms/InstCombine/objsize.ll (original)
> +++ llvm/trunk/test/Transforms/InstCombine/objsize.ll Thu Jan  6 07:11:05 2011
> @@ -160,3 +160,19 @@
>   ret i32 %objsize
> }
> 
> +define i32 @test8(i32 %x) {
> +; CHECK: @test8
> +  %alloc = call noalias i8* @malloc(i32 %x) nounwind
> +  %objsize = call i32 @llvm.objectsize.i32(i8* %alloc, i1 false) nounwind readonly
> +; CHECK-NEXT: ret i32 %x
> +  ret i32 %objsize
> +}
> +
> +define i32 @test9(i32 %x) {
> +; CHECK: @test9
> +  %alloc = call noalias i8* @malloc(i32 %x) nounwind
> +  %gep = getelementptr inbounds i8* %alloc, i32 16
> +  %objsize = call i32 @llvm.objectsize.i32(i8* %gep, i1 false) nounwind readonly
> +; CHECK-NOT: ret i32 %x
> +  ret i32 %objsize
> +}
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits