[PATCH] D84391: [AMDGPU] Fix incorrect arch assert while setting up FlatScratchInit

Madhur Amilkanthwar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 23 07:59:24 PDT 2020


madhur13490 added a comment.

In D84391#2169391 <https://reviews.llvm.org/D84391#2169391>, @arsenm wrote:

> In D84391#2169292 <https://reviews.llvm.org/D84391#2169292>, @madhur13490 wrote:
>
> > In D84391#2169172 <https://reviews.llvm.org/D84391#2169172>, @arsenm wrote:
> >
> > > Needs test. Also not sure why we would have an assert for this here in the first place
> >
> >
> > In fact I like the assert. It guarantees that we're not building something insane. Moreover, should we refactor this code to something more readble? :
> >
> >   switch(llvm::AMDGPUSubTarget) {
> >   case GFX:8 {}
> >   case GFX:9: {}
> >   ...
> >   }
> >  
> >
> >
> > Only caveat is the usage of function `flatScratchIsPointer()`. We can get rid of its usage in its current form but in future we may want to add some code to it to take contextual decisions.
>
>
> I would like to purge all checks of the generation. The relevant bit is flatScratchIsPointer. The generation is redundant and potentially misleading.


I am not sure what does that mean. Do you agree with my way of refactoring or you're suggesting something else? With my approach there won't be any checks as I said we can get rid of `flatScratchIsPointer()` too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84391





More information about the llvm-commits mailing list