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

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a reviewer: jeroen.dobbelaere.
lebedev.ri added a comment.

In D100717#2697110 <https://reviews.llvm.org/D100717#2697110>, @nikic wrote:

> In D100717#2697099 <https://reviews.llvm.org/D100717#2697099>, @lebedev.ri wrote:
>
>> In D100717#2697087 <https://reviews.llvm.org/D100717#2697087>, @nikic wrote:
>>
>>> I'm uncomfortable with using TBAA for this purpose.
>>
>>
>>
>>> The problem is that not all languages use TBAA, simply because that does not match their aliasing model.
>>> They shouldn't be penalized because TBAA information gets reused for an unrelated purpose.
>>
>> How does this penalize them?
>> If they don't use TBAA then there simply won't be a TBAA info on that memcpy,
>> and we'll use the previous behavior of performing an integer op.
>
> It penalizes them because they //cannot// use TBAA metadata, because they don't use type-based alias analysis. So they don't have a way to provide the necessary information, without also opting into alias analysis behavior that does not match their language semantics.

Sure, but how does *this* patch penalize them?
Isn't that more about the removal of faulty `inttoptr(ptrtoint p) -> p` fold?

> It binds two things (type hints for memcpy and alias analysis) together that don't have any relation.



> I'm assuming here that it's not possible to use TBAA without also impacting alias analysis -- but I'm not really familiar with TBAA and maybe my assumption is simply wrong. Is it possible to specify `!tbaa.struct` metadata on memcpy's without affecting AA behavior in any way?



>>> Might it make sense to simply always use a pointer type here (assuming size matches of course)?
>>
>> No. If it was a pointer we should treat it as such, if it was an integer we should treat it as such.
>> We shouldn't introduce bogus ptr<->int casts that weren't there originally.
>
> Just to clarify my thinking here: While we cannot fold `inttoptr(ptrtoint p) -> p`, we can fold `ptrtoint(inttoptr p) -> p`, so always doing the transfer as a pointer would avoid issues if the former fold is removed, because the latter would still eliminate cast pairs if it turns out to be the wrong type.

Both don't really sound sound to me, but i may be wrong.
Regardless, this seems like it will create polarly opposite problem to those fixed in referenced patches.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100717