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

[Juneyoung Lee via Phabricator via llvm-commits]

aqjune added a comment.

> 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. 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?

TBAA is used because otherwise there is no way to preserve the source’s data type down to middle-end optimizations.
To avoid the unwanted alias analysis, I think we have following options:

1. Use `unsigned char` for all other fields in tbaa_struct: `unsigned char` pointer is supposed to alias any pointer in C/C++, so it won't hurt.
2. Update optimization pipeline to never run TypeBasedAliasAnalysis for specific frontends.

If the language is supposed to add their own !tbaa/!tbaa_struct support, I guess either

1. TypeBasedAliasAnalysis can be extended to be more pluggable (allow one to add its own rule)
2. The language’s own TypeBasedAliasAnalysis can be just implemented and used instead.

> Might it make sense to simply always use a pointer type here (assuming size matches of course)?



> 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.

I also think it is desirable to preserve the original pointee type; it's because it again brings the complexity to the semantics of load/store.
Also, it is possible to introduce inttoptr again:

  store i64 %i, i64* %p
  memcpy(q, p, 8)
  ->
  store i64 %i, i64* %p
  %j = load i8* %p
  store i8* %j, i8** %q
  ->
  store i64 %i, i64* %p
  %j = inttoptr %i to i8*
  store i8* j, i8** %q

Existence of inttoptr complicates analyses to many attributes, e.g. nocapture/nofree/etc, so let's move towards avoiding introduction of them. :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100717