Aliasing caused by MemCpyOptimizer (PR18304)

Hello all,

PR18304 show an example where MemCpyOptimizer can cause pointers to
alias. Adding a requirement that the called function does not capture
the source pointer fixes the issue but it will also decrease the
number of opportunities for the transform.

Does this seem like the correct direction for a fix?
Will it be too restrictive?

Among the regression tests the proposed change causes failures in the
following files:
LLVM :: Transforms/MemCpyOpt/2008-02-24-MultipleUseofSRet.ll
LLVM :: Transforms/MemCpyOpt/2008-03-13-ReturnSlotBitcast.ll
LLVM :: Transforms/MemCpyOpt/loadstore-sret.ll
LLVM :: Transforms/MemCpyOpt/memcpy.ll
LLVM :: Transforms/MemCpyOpt/sret.ll

If the solution seems correct my idea is to add the "nocapture"
attribute to the functions called in the test files (assuming they
don't capture the source pointer) to get the same test behavior as
before.

Best regards
David

David Wiberg wrote:

Hello all,

PR18304 show an example where MemCpyOptimizer can cause pointers to
alias. Adding a requirement that the called function does not capture
the source pointer fixes the issue but it will also decrease the
number of opportunities for the transform.

Does this seem like the correct direction for a fix?
Will it be too restrictive?

It's the correct direction, and also correctness trumps optimization power so it doesn't matter whether it's too restrictive.

The only way we can fold away the memcpy is if both sides promise to not write to it before the other, and if both sides won't look at the pointer address itself. You can use "nocapture" to mean the latter. We can use "readonly" and "readnone" attributes for the former, but I don't think memcpyopt currently makes use of them when they appear on the function.

For instance, assuming %p1 points to a copy of what %p2 points to:

call @foo(i8* readonly %p1) ;; may capture %p1 but won't write to it.
call @bar(i8* nocapture %p2) ;; may modify %p2 but won't compare its address with anything, not even a copy captured by @foo

We can still merge %p1 and %p2 if these are the only uses.

Nick

I'm not sure about LLVM coding/patch standards but I imagine that adding
a regression test in the patch might be nice.

Best regards