[llvm-commits] [llvm] r46393 - in /llvm/trunk: lib/Transforms/Utils/InlineFunction.cpp test/CFrontend/2008-01-26-ReadOnlyByVal.c

[Chris Lattner]

On Jan 25, 2008, at 10:41 PM, Duncan Sands wrote:

> Author: baldrick
> Date: Sat Jan 26 00:41:49 2008
> New Revision: 46393
>
> URL: http://llvm.org/viewvc/llvm-project?rev=46393&view=rev
> Log:
> Create an explicit copy for byval parameters even
> when inlining a readonly function.

Are you sure this is correct?  There is a difference between 'const',  
'pure' and readnone/readonly.  Further, isn't it always safe to elide  
the copy for readnone functions?

-Chris

>
>
> Added:
>    llvm/trunk/test/CFrontend/2008-01-26-ReadOnlyByVal.c
> Modified:
>    llvm/trunk/lib/Transforms/Utils/InlineFunction.cpp
>
> Modified: llvm/trunk/lib/Transforms/Utils/InlineFunction.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/InlineFunction.cpp?rev=46393&r1=46392&r2=46393&view=diff
>
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- llvm/trunk/lib/Transforms/Utils/InlineFunction.cpp (original)
> +++ llvm/trunk/lib/Transforms/Utils/InlineFunction.cpp Sat Jan 26  
> 00:41:49 2008
> @@ -240,12 +240,15 @@
>          E = CalledFunc->arg_end(); I != E; ++I, ++AI, ++ArgNo) {
>       Value *ActualArg = *AI;
>
> -      // When byval arguments actually inlined, we need to make the  
> copy implied
> -      // by them explicit.  However, we don't do this if the callee  
> is readonly
> -      // or readnone, because the copy would be unneeded: the  
> callee doesn't
> -      // modify the struct.
> -      if (CalledFunc->paramHasAttr(ArgNo+1, ParamAttr::ByVal) &&
> -          !CalledFunc->onlyReadsMemory()) {
> +      // When byval arguments are inlined, we need to make the copy  
> implied
> +      // by them explicit.  It is tempting to think that this is  
> not needed if
> +      // the callee is readonly, because the callee doesn't modify  
> the struct.
> +      // However this would be wrong: readonly means that any  
> writes the callee
> +      // performs are not visible to the caller.  But writes by the  
> callee to
> +      // an argument passed byval are by definition not visible to  
> the caller!
> +      // Since we allow this kind of readonly function, there needs  
> to be an
> +      // explicit copy in order to keep the writes invisible after  
> inlining.
> +      if (CalledFunc->paramHasAttr(ArgNo+1, ParamAttr::ByVal)) {
>         const Type *AggTy = cast<PointerType>(I->getType())- 
> >getElementType();
>         const Type *VoidPtrTy = PointerType::getUnqual(Type::Int8Ty);
>
>
> Added: llvm/trunk/test/CFrontend/2008-01-26-ReadOnlyByVal.c
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CFrontend/2008-01-26-ReadOnlyByVal.c?rev=46393&view=auto
>
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- llvm/trunk/test/CFrontend/2008-01-26-ReadOnlyByVal.c (added)
> +++ llvm/trunk/test/CFrontend/2008-01-26-ReadOnlyByVal.c Sat Jan 26  
> 00:41:49 2008
> @@ -0,0 +1,19 @@
> +// RUN: %llvmgcc %s -S -O1 -o - | llvm-as | opt -std-compile-opts |  
> llvm-dis | not grep add
> +
> +struct S { int A; int B; int C; int D; };
> +
> +int f(struct S x) __attribute__ ((const));
> +
> +static int __attribute__ ((const)) g(struct S x) {
> +	x.A = x.B;
> +	return f(x);
> +}
> +
> +int h(void) {
> +	struct S x;
> +	int r;
> +	x.A = 0;
> +	x.B = 9;
> +	r = g(x);
> +	return r + x.A;
> +}
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits