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

[Yaxun Liu via Phabricator via llvm-commits]

yaxunl added a comment.

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

> > ! In https://reviews.llvm.org/D39670#919711, @yaxunl wrote:
> > 
> > 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.
>
> I disagree with this, but it's also possible I'm wrong.  If you are willing to do the work and deal with the fallout, then I'm OK with this change.  Just make sure  @jvesely is aware so he can help test/check for regressions.


Sure. I will keep Jan updated.

> One problem I have with this specific test is you are essentially removing a test for the current mapping and replacing it with a test for the new mapping, even though the new mapping isn't actually being used anywhere outside of LLVM.  What is your timeframe for migrating r600 to the new mapping?

I plan to get it done ASAP. Hopefully in one month. I think the new address space mapping and old address space mapping share most of the code path in llvm/clang, so any regression for the old address space mapping is likely causes regression in the new address space mapping. On the other hand, since new address space mapping has non-zero alloca address space therefore some extra code path, regressions in new address space mapping may not cause regressions in old address space mapping.


https://reviews.llvm.org/D39670