Hi Owen, nice transform! > + // Check that we're copying to an argument... > + Value* cpyDest = cpy->getDest(); > + if (!isa<Argument>(cpyDest)) > + return false; > + > + // And that the argument is the return slot > + Argument* sretArg = cast<Argument>(cpyDest); > + if (!sretArg->hasStructRetAttr()) > + return false;
why is it relevant that the memcpy is to an sret argument? The transform would be equally valid if it was to a temporary I think. The essential condition seems to be: the memcpy destination should not be accessable (for read or write) by the function called. Knowing that the destination is a noalias parameter is simply helpful in determining that (I don't see what sret has to do with it...). On the other hand, it matters that the memcpy source is passed to an sret parameter, because this means that the called function does not read any existing values in it (i.e. it is undefined what the callee gets if it tries to read existing values via the sret parameter). OK, I know this is not explicitly part of the sret definition, but it could reasonably be. The patch doesn't seem to check whether the memcpy source is being passed as an sret parameter. Finally, it matters that the size of the memcpy is the whole size of the memcpy source. Imagine that you pass a 10 byte memtmp to the call, then memcpy 5 bytes of it to a 5 byte object. It is clearly wrong to pass the 5 byte object directly to the call. Ciao, Duncan. PS: The other conditions like no other uses also seem way too strict. _______________________________________________ llvm-commits mailing list llvm-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
