[PATCH] D76107: AMDGPU: Don't handle kernarg.segment.ptr in functions

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec accepted this revision.
rampitec added a comment.
This revision is now accepted and ready to land.

In D76107#1920749 <https://reviews.llvm.org/D76107#1920749>, @arsenm wrote:

> In D76107#1920704 <https://reviews.llvm.org/D76107#1920704>, @rampitec wrote:
>
> > A trap would be better, we can easily spot it.
>
>
>
>
> In D76107#1920704 <https://reviews.llvm.org/D76107#1920704>, @rampitec wrote:
>
> > A trap would be better, we can easily spot it.
>
>
> A trap isn’t particularly different from the null dereference if you try to actually use it. I was leaning more towards a compile time error


It is actually very different. A null pointer dereference means debugging, tracking the pointer, analyzing IR step by step until you see why was null emitted. A trap on the contrary is immediately visible in the ISA dump and immediately triggers the alarm, that compiler have spotted the code which should never been executed.

In D76107#1921570 <https://reviews.llvm.org/D76107#1921570>, @arsenm wrote:

> In D76107#1920799 <https://reviews.llvm.org/D76107#1920799>, @rampitec wrote:
>
> > In D76107#1920749 <https://reviews.llvm.org/D76107#1920749>, @arsenm wrote:
> >
> > > In D76107#1920704 <https://reviews.llvm.org/D76107#1920704>, @rampitec wrote:
> > >
> > > > A trap would be better, we can easily spot it.
> > >
> > >
> > >
> > >
> > > In D76107#1920704 <https://reviews.llvm.org/D76107#1920704>, @rampitec wrote:
> > >
> > > > A trap would be better, we can easily spot it.
> > >
> > >
> > > A trap isn’t particularly different from the null dereference if you try to actually use it. I was leaning more towards a compile time error
> >
> >
> > It is actually very different. A null pointer dereference means debugging, tracking the pointer, analyzing IR step by step until you see why was null emitted. A trap on the contrary is immediately visible in the ISA dump and immediately triggers the alarm, that compiler have spotted the code which should never been executed.
>
>
> The trap adds extra complexity to the implementation since now the queue ptr needs to be passed to the function. This should probably be considered a backend internal intrinsic now, so I'm not sure it's worth the effort


Sigh. OK, let's hope we will never have to debug it.


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

https://reviews.llvm.org/D76107