[PATCH] D108315: [AMDGPU] Add alias.scope metadata to lowered LDS struct

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec added a comment.

In D108315#2953690 <https://reviews.llvm.org/D108315#2953690>, @JonChesterfield wrote:

> Change looks good but this comment is strange:
>
>   // Create alias.scope and their lists. Each field in the new structure
>   // does not alias with all other fields.
>
> That is true of any structure. Each s/field/pointer in the new?
>
> It seems a shame that moving variables into a struct thwarts AA at present.

That is all true, but unfortunately... it is true. I have tried to create a struct upfront and clang didn't produce this sort of metadata. And no AA was able to disambiguate pointers to fields. So we will be in a better position with this change than anybody who just have created a structure in the source. I do agree, this is a shame. Maybe if FE would create this sort of AA metadata that is a solution, maybe yet another AA pass is. Right now structs are a problem.

One of the benefits of combining LDS into a single aggregate was to combine loads and stores because we know the layout of LDS. And that turns to be not working. Well, I hope this will help at least that problem. Sure anough if there is an AA pass to disambiguate struct fields this patch will become unnecessary, but I am afraid in the existence of offsetof operator that might not happen. I can see a very easy way to exploit it. In fact any assumed layout combined with an ability to take and cast pointers is.


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

https://reviews.llvm.org/D108315