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@cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits _______________________________________________ llvm-commits mailing list llvm-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
