[PATCH] Gut InstCombiner::SimplifyMemTransfer.

[John McCall via llvm-commits]

> On Aug 18, 2015, at 5:01 PM, David Majnemer <david.majnemer at gmail.com> wrote:
> On Tue, Aug 18, 2015 at 4:57 PM, John McCall <rjmccall at apple.com <mailto:rjmccall at apple.com>> wrote:
> 
> > On Aug 16, 2015, at 3:08 AM, Hal Finkel <hfinkel at anl.gov <mailto:hfinkel at anl.gov>> wrote:
> >
> > [+Richard, John, Chandler]
> >
> > ----- Original Message -----
> >> From: "David Majnemer via llvm-commits" <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>>
> >> To: "Lang Hames" <lhames at gmail.com <mailto:lhames at gmail.com>>
> >> Cc: "Commit Messages and Patches for LLVM" <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>>
> >> Sent: Wednesday, August 12, 2015 10:35:16 PM
> >> Subject: Re: [PATCH] Gut InstCombiner::SimplifyMemTransfer.
> >>
> >>
> >>
> >> Hi Lang,
> >>
> >>
> >> I reviewed the InstCombine side and it looks great, all that I ask is
> >> that you replace the `Instruction *` with `auto *` because the type
> >> is replicated in the `dyn_cast`.
> >>
> >>
> >> My only concern is that clang will actually create calls to the
> >> memcpy intrinsic from within clang with pointers which exactly
> >> alias:
> >> http://clang.llvm.org/doxygen/CGExprAgg_8cpp_source.html#l01448 <https://urldefense.proofpoint.com/v2/url?u=http-3A__clang.llvm.org_doxygen_CGExprAgg-5F8cpp-5Fsource.html-23l01448&d=BQMFaQ&c=eEvniauFctOgLOKGJOplqw&r=gQ44wRp_9SsWkoyjg4iciq3zFxOsNPj8T2uXNJZw7Z4&m=IaBwqeYYpEEeJWOgiIt_iFa1UrMa3qF5GTvjNp01Hs0&s=gqxTvFO4cMW6ojGEjcS4Ms5lV7ZqS_4xQVtm5IBnxKE&e=>
> >>
> >
> > That seems very broken. We need to fix this. In fact, the comment admits as much:
> >
> > 01454   // memcpy is not defined if the source and destination pointers are exactly
> > 01455   // equal, but other compilers do this optimization, and almost every memcpy
> > 01456   // implementation handles this case safely.  If there is a libc that does not
> > 01457   // safely handle this, we can add a target hook.
> >
> > Is there any reason not to emit memmove instead?
> 
> Partial overlaps are generally not possible here — C guarantees that the overlap
> in an assignment has to be perfect.  So a great deal of the added complexity of
> memmove is not needed; and as mentioned, no actual implementations of
> memcpy misbehave for perfect overlap.
> 
> That said, I can appreciate wanting to keep the formal model sane.  I suspect that
> emitting llvm.memmove would generally produce uses of the actual memmove,
> but I'd be happy to use a different intrinsic that allows only perfect overlap and which
> you can make a “platform-specific” decision to lower to memcpy in the backend.
> 
> I suggested to Lang on IRC that we might want to add another operand to the memcpy intrinsic to model this behavior.  Another intrinsic would, of course, work just as well.  AFAICT, the only code which would care about the distinction between memcpy as-written-in-source and as-written-by-compiler which would make the extra flag a little less disruptive than another intrinsic.

Oh, by the way, if you’re going to revise or embellish these intrinsics, please also allow separate alignments to be expressed for both operands.

John.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150818/3456a220/attachment.html>