[PATCH] D88788: [SROA] rewritePartition()/findCommonType(): if uses have conflicting type, try getTypePartition() before falling back to largest integral use type (PR47592)

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

In D88788#2313958 <https://reviews.llvm.org/D88788#2313958>, @nlopes wrote:

> In D88788#2311004 <https://reviews.llvm.org/D88788#2311004>, @efriedma wrote:
>
>> I guess if we're relying on the allocated type of the alloca anyway, preferring it over an integer type isn't terrible.
>>
>> Really, though, we should avoid relying on the allocated type where possible.  Here, we could check if any of the load/store operations use a pointer type, and choose a pointer type in that case.
>
> Agreed. But until LLVM removes pointer sub-types it's convenient to get the alloca type right to avoid bitcast on every access anyway.
> When pointer sub-types goes away, I guess all this code in SROA to find the right type for alloca would go away, but as you say it would have to be replaced with code to get the right load/store type instead. (FWIW Alive2's alloca only takes the number of bytes to allocate as argument)
> So I see this patch as a step in the right direction.

So i don't do this blindly to find out that is'a bad idea, can we agree on the baseline here?
How should this be done properly? Instead of relying on the allocation type, would the D88842 <https://reviews.llvm.org/D88842>'s approach be applicable here?

I've looked now that rGe00f189d392d <https://reviews.llvm.org/rGe00f189d392dd9bf95f6a98f05f2d341d06cd65c> is done, and i see two-ish main origin sources of introduction inttoptr/ptrtoint that weren't there before:

1. instcombine's `SimplifyAnyMemTransfer()` - given `memcpy` of a pointer-sized thingy, we lower it to a load+store of an integer
2. instcombine's `combineLoadToOperationType()` - `CastInst::isNoopCast()` needs to be updated that these casts are not no-op. (`isEliminableCastPair()` too, i guess)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88788