[PATCH] D100717: [InstCombine] Transform memcpy to ptr load/stores if TBAA says so

Wed Apr 21 23:14:29 PDT 2021

jeroen.dobbelaere added a comment.

In D100717#2706600 <https://reviews.llvm.org/D100717#2706600>, @lebedev.ri wrote:

> In D100717#2706571 <https://reviews.llvm.org/D100717#2706571>, @nikic wrote:
>
>> Once the LangRef changes are there, I'd suggest cross-posting this to llvm-dev for visibility. This patch was discussed at the last alias analysis meeting,
>
> (hm, was there no reminder mail, or did i miss it?)
>
>> and I think most people were in favor of using metadata for this purpose, but weren't super fond of determining the type to use based on string matching. Maybe someone else has more detailed notes...
>
> To be noted, isn't really inventing a new approach though, it's already done for vtable pointers.
> I feel like that if this isn't ok, then the existing code also should be removed.

It is done for vtable pointers, but that is only used by the thread sanitizer. There was discussion about using the available information in tbaa for guiding the type to load/store (1) vs forcing to use tbaa to guide what kind of load/stores are used. (2)

(1) is deemed to be acceptable, if it is clearly documented. The forced naming based on what names clang produces is unfortunate, but as a stopgap this would be acceptable until we come up with a cleaner mechanism.
That cleaner mechanism could be part of the rework of `new struct path tbaa` (that was also discussed at the meeting).

For (2) we don't have a good idea yet. Maybe a separate `!tb.struct` with the same layout could be used for this particular purpose if `!tbaa.struct` is not available (just thinking out loud).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100717/new/

https://reviews.llvm.org/D100717