[PATCH] D39670: [AMDGPU] Fix pointer info for pseudo source for r600

Yaxun Liu via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 8 11:57:20 PST 2017


yaxunl added a comment.

In https://reviews.llvm.org/D39670#918904, @tstellar wrote:

> In https://reviews.llvm.org/D39670#918887, @yaxunl wrote:
>
> > We discussed this internally and concluded that having a static address space mapping is more important. Most of issues are due to using dummy pointer info. Such places should be few in r600 code, therefore we will continue fixing these issues unless we found there need excessive efforts to do so.
>
>
> It would be nice to have these discussions on the mailing list so more people could participate, and it would be helpful for convincing people like me that this is the right approach, but as of right now I don't see why it is so important to have a static address space mapping that is the same for r600 and amdgcn.  I think the better approach would be to share less code between the two subtargets such that it's possible for each to have their own static mapping.  I think this kind of code separation is something should be done anyway independent of address space mapping work.


As Matt said, the risk of having two sets of address space mapping is high. Also I estimate the workload to switch r600 to the new addr space mapping is moderate. On the hand, the workload for separating r600 from amdgcn is high.

In https://reviews.llvm.org/D39670#918904, @tstellar wrote:

> In https://reviews.llvm.org/D39670#918887, @yaxunl wrote:
>
> > We discussed this internally and concluded that having a static address space mapping is more important. Most of issues are due to using dummy pointer info. Such places should be few in r600 code, therefore we will continue fixing these issues unless we found there need excessive efforts to do so.
>
>
> It would be nice to have these discussions on the mailing list so more people could participate, and it would be helpful for convincing people like me that this is the right approach, but as of right now I don't see why it is so important to have a static address space mapping that is the same for r600 and amdgcn.  I think the better approach would be to share less code between the two subtargets such that it's possible for each to have their own static mapping.  I think this kind of code separation is something should be done anyway independent of address space mapping work.


r600 and amdgcn share lots of passes. It is not practical to duplicate these passes and maintain them separately. Even if we were able to do that, maintaining two sets of static address space is error prone. On the other hand, let r600 switching to the new address space mapping takes only moderate efforts since it is only address space number change. We will fix the regressions arising from this change. After this change, we will get a more consistent and bug-free AMDGPU backend.


https://reviews.llvm.org/D39670





More information about the llvm-commits mailing list