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

Mon Apr 19 00:22:00 PDT 2021

jeroen.dobbelaere added a subscriber: jdoerfert.
jeroen.dobbelaere added a comment.

In D100717#2697137 <https://reviews.llvm.org/D100717#2697137>, @aqjune wrote:

> In D100717#2697125 <https://reviews.llvm.org/D100717#2697125>, @fhahn wrote:
>
>> I don't see how this use of TBAA metadata is within the specification of the metadata.
>>
>> IIUC the type descriptors only encode the layout and relationship between types, but not whether a scalar type is a pointer, integer, floating point or other type. It just happens to be that Clang uses names that are related to the source types. But IIUC those names are arbitrary and Clang could just as well use `A`, `B`, `C`,... or any other naming scheme.
>>
>> Please let me know if I am missing something that provides the guarantees the patch uses.
>
> For metadata nodes whose descriptions are simply C/C++'s type name they shouldn't carry info, but for those with special descriptions would it be reasonable if they are given special meaning?
> I thought TBAA used the description because `MDNode::isTBAAVtableAccess` checks whether description is `"vtable pointer"`.

The `vtable pointer` case seems only to be used by the `ThreadSanitizer`.

I am not opposed to provide special meaning to certain nodes, but this should be mentioned in the documentation. Currently `vtable pointer` and `any pointer` are not documented as special descriptors in the langref's tbaa/tbaa.struct chapters.

@jdoerfert what do you think ?

I am not sure what kind of complications to expect when you have pointers pointing to a non-0 address space. These can differ in size and representation, but that should be taken care of by the size check.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100717