Re: [Mesa-dev] [PATCH] nvc0/ir: Fix assert accessing null pointer
On Fri, 2019-07-26 at 10:41 -0400, Ilia Mirkin wrote: > On Wed, Jul 24, 2019 at 9:34 AM Juan A. Suarez Romero > wrote: > > On Wed, 2019-07-24 at 14:27 +0200, Karol Herbst wrote: > > > it's only fixing a crash in a build with asserts enabled, but if > > > somebody wants to apply those to stable, then go ahead. > > > > > > > OK; in that case I will keep it out. > > Looks like distros build with debug enabled, sadly: > > https://bugs.freedesktop.org/show_bug.cgi?id=111218 > > Please do include this in stable: All right! I'll include it then. Thanks! J.A. > > commit 7493fbf032f5bcbf4c48187bc089c9a34f04a1d5 > Author: Mark Menzynski > Date: Fri Jul 19 13:09:02 2019 +0200 > > nvc0/ir: Fix assert accessing null pointer > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111007 > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=67 > > Signed-off-by: Mark Menzynski > Reviewed-by: Ilia Mirkin > Reviewed-by: Tobias Klausmann > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] nvc0/ir: Fix assert accessing null pointer
On Wed, Jul 24, 2019 at 9:34 AM Juan A. Suarez Romero wrote: > > On Wed, 2019-07-24 at 14:27 +0200, Karol Herbst wrote: > > it's only fixing a crash in a build with asserts enabled, but if > > somebody wants to apply those to stable, then go ahead. > > > > OK; in that case I will keep it out. Looks like distros build with debug enabled, sadly: https://bugs.freedesktop.org/show_bug.cgi?id=111218 Please do include this in stable: commit 7493fbf032f5bcbf4c48187bc089c9a34f04a1d5 Author: Mark Menzynski Date: Fri Jul 19 13:09:02 2019 +0200 nvc0/ir: Fix assert accessing null pointer Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111007 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=67 Signed-off-by: Mark Menzynski Reviewed-by: Ilia Mirkin Reviewed-by: Tobias Klausmann ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] nvc0/ir: Fix assert accessing null pointer
On Wed, 2019-07-24 at 14:27 +0200, Karol Herbst wrote:
> it's only fixing a crash in a build with asserts enabled, but if
> somebody wants to apply those to stable, then go ahead.
>
OK; in that case I will keep it out.
Thanks!
J.A.
> On Wed, Jul 24, 2019 at 12:48 PM Juan A. Suarez Romero
> wrote:
> > On Fri, 2019-07-19 at 13:56 +0200, Mark Menzynski wrote:
> > > Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=111007
> > > Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=67
> > > Signed-off-by: Mark Menzynski
> > > ---
> > > src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > Looks like a good candidate for 19.1 stable. WDYT?
> >
> > J.A.
> >
> > > diff --git
> > > a/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
> > > b/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
> > > index aca3b0afb1e..1f702a987d8 100644
> > > --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
> > > +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
> > > @@ -51,12 +51,12 @@ NVC0LegalizeSSA::handleDIV(Instruction *i)
> > > // Generate movs to the input regs for the call we want to generate
> > > for (int s = 0; i->srcExists(s); ++s) {
> > >Instruction *ld = i->getSrc(s)->getInsn();
> > > - assert(ld->getSrc(0) != NULL);
> > >// check if we are moving an immediate, propagate it in that case
> > >if (!ld || ld->fixed || (ld->op != OP_LOAD && ld->op != OP_MOV) ||
> > > !(ld->src(0).getFile() == FILE_IMMEDIATE))
> > > bld.mkMovToReg(s, i->getSrc(s));
> > >else {
> > > + assert(ld->getSrc(0) != NULL);
> > > bld.mkMovToReg(s, ld->getSrc(0));
> > > // Clear the src, to make code elimination possible here before
> > > we
> > > // delete the instruction i later
> >
> > ___
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] nvc0/ir: Fix assert accessing null pointer
it's only fixing a crash in a build with asserts enabled, but if
somebody wants to apply those to stable, then go ahead.
On Wed, Jul 24, 2019 at 12:48 PM Juan A. Suarez Romero
wrote:
>
> On Fri, 2019-07-19 at 13:56 +0200, Mark Menzynski wrote:
> > Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=111007
> > Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=67
> > Signed-off-by: Mark Menzynski
> > ---
> > src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
>
>
> Looks like a good candidate for 19.1 stable. WDYT?
>
> J.A.
>
> >
> > diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
> > b/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
> > index aca3b0afb1e..1f702a987d8 100644
> > --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
> > +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
> > @@ -51,12 +51,12 @@ NVC0LegalizeSSA::handleDIV(Instruction *i)
> > // Generate movs to the input regs for the call we want to generate
> > for (int s = 0; i->srcExists(s); ++s) {
> >Instruction *ld = i->getSrc(s)->getInsn();
> > - assert(ld->getSrc(0) != NULL);
> >// check if we are moving an immediate, propagate it in that case
> >if (!ld || ld->fixed || (ld->op != OP_LOAD && ld->op != OP_MOV) ||
> > !(ld->src(0).getFile() == FILE_IMMEDIATE))
> > bld.mkMovToReg(s, i->getSrc(s));
> >else {
> > + assert(ld->getSrc(0) != NULL);
> > bld.mkMovToReg(s, ld->getSrc(0));
> > // Clear the src, to make code elimination possible here before we
> > // delete the instruction i later
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] nvc0/ir: Fix assert accessing null pointer
On Fri, 2019-07-19 at 13:56 +0200, Mark Menzynski wrote:
> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=111007
> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=67
> Signed-off-by: Mark Menzynski
> ---
> src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Looks like a good candidate for 19.1 stable. WDYT?
J.A.
>
> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
> b/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
> index aca3b0afb1e..1f702a987d8 100644
> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
> @@ -51,12 +51,12 @@ NVC0LegalizeSSA::handleDIV(Instruction *i)
> // Generate movs to the input regs for the call we want to generate
> for (int s = 0; i->srcExists(s); ++s) {
>Instruction *ld = i->getSrc(s)->getInsn();
> - assert(ld->getSrc(0) != NULL);
>// check if we are moving an immediate, propagate it in that case
>if (!ld || ld->fixed || (ld->op != OP_LOAD && ld->op != OP_MOV) ||
> !(ld->src(0).getFile() == FILE_IMMEDIATE))
> bld.mkMovToReg(s, i->getSrc(s));
>else {
> + assert(ld->getSrc(0) != NULL);
> bld.mkMovToReg(s, ld->getSrc(0));
> // Clear the src, to make code elimination possible here before we
> // delete the instruction i later
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] nvc0/ir: Fix assert accessing null pointer
On Friday, 2019-07-19 21:23:24 +0200, Tobias Klausmann wrote:
>
> Am 19.07.19 um 18:18 schrieb Ilia Mirkin:
> > On Fri, Jul 19, 2019 at 12:07 PM Tobias Klausmann
> > wrote:
> > > On 19.07.19 15:39, Eric Engestrom wrote:
> > > > On Friday, 2019-07-19 13:56:30 +0200, Mark Menzynski wrote:
> > > > > Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=111007
> > > > > Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=67
> > > > `Fixes:` is used to indicate the commit that introduced the code being
> > > > fixed, such as:
> > > > Fixes: 1c4e6d7ca83578caf521 ("nvc0/ir: propagate immediates to CALL
> > > > input MOVs")
> > > > This tag is used by our tools to backport fixes to the relevant stable
> > > > releases.
> > > >
> > > > Bugzilla entries are referenced using the `Bugzilla:` prefix.
> > > >
> > > > > Signed-off-by: Mark Menzynski
> > > > > ---
> > > > >src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp | 2
> > > > > +-
> > > > >1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > diff --git
> > > > > a/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
> > > > > b/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
> > > > > index aca3b0afb1e..1f702a987d8 100644
> > > > > --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
> > > > > +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
> > > > > @@ -51,12 +51,12 @@ NVC0LegalizeSSA::handleDIV(Instruction *i)
> > > > > // Generate movs to the input regs for the call we want to
> > > > > generate
> > > > > for (int s = 0; i->srcExists(s); ++s) {
> > > > > Instruction *ld = i->getSrc(s)->getInsn();
> > > > > - assert(ld->getSrc(0) != NULL);
> > > > I'll admit I don't know anything about this code, but it looks like
> > > > this might be a better fix?
> > > > assert(ld == NULL || ld->getSrc(0) != NULL)
> > > >
> > > > I cc'ed Tobias who wrote this code as he'll probably know best.
> > > Thanks for letting me know, but i'm kind of out of the loop and sadly
> > > don't remember too much about this one.
> > >
> > > Yet while having a look at the code i somehow wonder if there is a
> > > possibility to have
> > >
> > > if (!ld || ld->fixed || (ld->op != OP_LOAD && ld->op != OP_MOV)
> > > ||
> > >!(ld->src(0).getFile() == FILE_IMMEDIATE))
> > >
> > > crash at ld->src(0), as this is only set when a value is added to the
> > > instruction. So in the case ld->src(0) is legal, ld->getSrc(0) should be
> > > as well and we would need the assert at the beginning...
> > >
> > > PS: Did a functional change bring this to the light? (I ran piglit in
> > > front of every patch sumbission :/)
> > Nope. Just weird shaders that divide and/or mod 0 by a non-constant amount.
> >
> > There's an argument to just remove that assert entirely - not sure
> > that it's adding a whole lot, except complication.
>
> Ok,
>
> in this case i'm happy with the patch, with the Bugzilla References and the
> proper Fixes tag added this is:
>
> Reviewed-by: Tobias Klausmann
> (yes this mail address is different, but my uni mail address won't exist for
> much longer)
You might want to add a line to the .mailmap then ;)
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] nvc0/ir: Fix assert accessing null pointer
Am 19.07.19 um 18:18 schrieb Ilia Mirkin:
On Fri, Jul 19, 2019 at 12:07 PM Tobias Klausmann
wrote:
On 19.07.19 15:39, Eric Engestrom wrote:
On Friday, 2019-07-19 13:56:30 +0200, Mark Menzynski wrote:
Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=111007
Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=67
`Fixes:` is used to indicate the commit that introduced the code being
fixed, such as:
Fixes: 1c4e6d7ca83578caf521 ("nvc0/ir: propagate immediates to CALL input
MOVs")
This tag is used by our tools to backport fixes to the relevant stable
releases.
Bugzilla entries are referenced using the `Bugzilla:` prefix.
Signed-off-by: Mark Menzynski
---
src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
b/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
index aca3b0afb1e..1f702a987d8 100644
--- a/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
+++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
@@ -51,12 +51,12 @@ NVC0LegalizeSSA::handleDIV(Instruction *i)
// Generate movs to the input regs for the call we want to generate
for (int s = 0; i->srcExists(s); ++s) {
Instruction *ld = i->getSrc(s)->getInsn();
- assert(ld->getSrc(0) != NULL);
I'll admit I don't know anything about this code, but it looks like
this might be a better fix?
assert(ld == NULL || ld->getSrc(0) != NULL)
I cc'ed Tobias who wrote this code as he'll probably know best.
Thanks for letting me know, but i'm kind of out of the loop and sadly
don't remember too much about this one.
Yet while having a look at the code i somehow wonder if there is a
possibility to have
if (!ld || ld->fixed || (ld->op != OP_LOAD && ld->op != OP_MOV) ||
!(ld->src(0).getFile() == FILE_IMMEDIATE))
crash at ld->src(0), as this is only set when a value is added to the instruction. So
in the case ld->src(0) is legal, ld->getSrc(0) should be as well and we would need
the assert at the beginning...
PS: Did a functional change bring this to the light? (I ran piglit in front of
every patch sumbission :/)
Nope. Just weird shaders that divide and/or mod 0 by a non-constant amount.
There's an argument to just remove that assert entirely - not sure
that it's adding a whole lot, except complication.
Ok,
in this case i'm happy with the patch, with the Bugzilla References and
the proper Fixes tag added this is:
Reviewed-by: Tobias Klausmann
(yes this mail address is different, but my uni mail address won't exist for
much longer)
Thanks,
Tobias
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] nvc0/ir: Fix assert accessing null pointer
On Fri, Jul 19, 2019 at 12:07 PM Tobias Klausmann
wrote:
> On 19.07.19 15:39, Eric Engestrom wrote:
> > On Friday, 2019-07-19 13:56:30 +0200, Mark Menzynski wrote:
> >> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=111007
> >> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=67
> > `Fixes:` is used to indicate the commit that introduced the code being
> > fixed, such as:
> >Fixes: 1c4e6d7ca83578caf521 ("nvc0/ir: propagate immediates to CALL
> > input MOVs")
> > This tag is used by our tools to backport fixes to the relevant stable
> > releases.
> >
> > Bugzilla entries are referenced using the `Bugzilla:` prefix.
> >
> >> Signed-off-by: Mark Menzynski
> >> ---
> >> src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
> >> b/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
> >> index aca3b0afb1e..1f702a987d8 100644
> >> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
> >> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
> >> @@ -51,12 +51,12 @@ NVC0LegalizeSSA::handleDIV(Instruction *i)
> >> // Generate movs to the input regs for the call we want to generate
> >> for (int s = 0; i->srcExists(s); ++s) {
> >> Instruction *ld = i->getSrc(s)->getInsn();
> >> - assert(ld->getSrc(0) != NULL);
> > I'll admit I don't know anything about this code, but it looks like
> > this might be a better fix?
> > assert(ld == NULL || ld->getSrc(0) != NULL)
> >
> > I cc'ed Tobias who wrote this code as he'll probably know best.
>
> Thanks for letting me know, but i'm kind of out of the loop and sadly
> don't remember too much about this one.
>
> Yet while having a look at the code i somehow wonder if there is a
> possibility to have
>
> if (!ld || ld->fixed || (ld->op != OP_LOAD && ld->op != OP_MOV) ||
> !(ld->src(0).getFile() == FILE_IMMEDIATE))
>
> crash at ld->src(0), as this is only set when a value is added to the
> instruction. So in the case ld->src(0) is legal, ld->getSrc(0) should be as
> well and we would need the assert at the beginning...
>
> PS: Did a functional change bring this to the light? (I ran piglit in front
> of every patch sumbission :/)
Nope. Just weird shaders that divide and/or mod 0 by a non-constant amount.
There's an argument to just remove that assert entirely - not sure
that it's adding a whole lot, except complication.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] nvc0/ir: Fix assert accessing null pointer
On 19.07.19 15:39, Eric Engestrom wrote:
On Friday, 2019-07-19 13:56:30 +0200, Mark Menzynski wrote:
Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=111007
Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=67
`Fixes:` is used to indicate the commit that introduced the code being
fixed, such as:
Fixes: 1c4e6d7ca83578caf521 ("nvc0/ir: propagate immediates to CALL input
MOVs")
This tag is used by our tools to backport fixes to the relevant stable
releases.
Bugzilla entries are referenced using the `Bugzilla:` prefix.
Signed-off-by: Mark Menzynski
---
src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
b/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
index aca3b0afb1e..1f702a987d8 100644
--- a/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
+++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
@@ -51,12 +51,12 @@ NVC0LegalizeSSA::handleDIV(Instruction *i)
// Generate movs to the input regs for the call we want to generate
for (int s = 0; i->srcExists(s); ++s) {
Instruction *ld = i->getSrc(s)->getInsn();
- assert(ld->getSrc(0) != NULL);
I'll admit I don't know anything about this code, but it looks like
this might be a better fix?
assert(ld == NULL || ld->getSrc(0) != NULL)
I cc'ed Tobias who wrote this code as he'll probably know best.
Thanks for letting me know, but i'm kind of out of the loop and sadly
don't remember too much about this one.
Yet while having a look at the code i somehow wonder if there is a
possibility to have
if (!ld || ld->fixed || (ld->op != OP_LOAD && ld->op != OP_MOV) ||
!(ld->src(0).getFile() == FILE_IMMEDIATE))
crash at ld->src(0), as this is only set when a value is added to the instruction. So
in the case ld->src(0) is legal, ld->getSrc(0) should be as well and we would need
the assert at the beginning...
PS: Did a functional change bring this to the light? (I ran piglit in front of
every patch sumbission :/)
Greetings,
Tobias
// check if we are moving an immediate, propagate it in that case
if (!ld || ld->fixed || (ld->op != OP_LOAD && ld->op != OP_MOV) ||
!(ld->src(0).getFile() == FILE_IMMEDIATE))
bld.mkMovToReg(s, i->getSrc(s));
else {
+ assert(ld->getSrc(0) != NULL);
bld.mkMovToReg(s, ld->getSrc(0));
// Clear the src, to make code elimination possible here before we
// delete the instruction i later
--
2.21.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] nvc0/ir: Fix assert accessing null pointer
The patch is correct as-is.
Reviewed-by: Ilia Mirkin
On Fri, Jul 19, 2019 at 9:39 AM Eric Engestrom wrote:
>
> On Friday, 2019-07-19 13:56:30 +0200, Mark Menzynski wrote:
> > Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=111007
> > Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=67
>
> `Fixes:` is used to indicate the commit that introduced the code being
> fixed, such as:
> Fixes: 1c4e6d7ca83578caf521 ("nvc0/ir: propagate immediates to CALL input
> MOVs")
> This tag is used by our tools to backport fixes to the relevant stable
> releases.
>
> Bugzilla entries are referenced using the `Bugzilla:` prefix.
>
> > Signed-off-by: Mark Menzynski
> > ---
> > src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
> > b/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
> > index aca3b0afb1e..1f702a987d8 100644
> > --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
> > +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
> > @@ -51,12 +51,12 @@ NVC0LegalizeSSA::handleDIV(Instruction *i)
> > // Generate movs to the input regs for the call we want to generate
> > for (int s = 0; i->srcExists(s); ++s) {
> >Instruction *ld = i->getSrc(s)->getInsn();
> > - assert(ld->getSrc(0) != NULL);
>
> I'll admit I don't know anything about this code, but it looks like
> this might be a better fix?
>assert(ld == NULL || ld->getSrc(0) != NULL)
>
> I cc'ed Tobias who wrote this code as he'll probably know best.
>
> >// check if we are moving an immediate, propagate it in that case
> >if (!ld || ld->fixed || (ld->op != OP_LOAD && ld->op != OP_MOV) ||
> > !(ld->src(0).getFile() == FILE_IMMEDIATE))
> > bld.mkMovToReg(s, i->getSrc(s));
> >else {
> > + assert(ld->getSrc(0) != NULL);
> > bld.mkMovToReg(s, ld->getSrc(0));
> > // Clear the src, to make code elimination possible here before we
> > // delete the instruction i later
> > --
> > 2.21.0
> >
> > ___
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] nvc0/ir: Fix assert accessing null pointer
On Friday, 2019-07-19 13:56:30 +0200, Mark Menzynski wrote:
> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=111007
> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=67
`Fixes:` is used to indicate the commit that introduced the code being
fixed, such as:
Fixes: 1c4e6d7ca83578caf521 ("nvc0/ir: propagate immediates to CALL input
MOVs")
This tag is used by our tools to backport fixes to the relevant stable
releases.
Bugzilla entries are referenced using the `Bugzilla:` prefix.
> Signed-off-by: Mark Menzynski
> ---
> src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
> b/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
> index aca3b0afb1e..1f702a987d8 100644
> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
> @@ -51,12 +51,12 @@ NVC0LegalizeSSA::handleDIV(Instruction *i)
> // Generate movs to the input regs for the call we want to generate
> for (int s = 0; i->srcExists(s); ++s) {
>Instruction *ld = i->getSrc(s)->getInsn();
> - assert(ld->getSrc(0) != NULL);
I'll admit I don't know anything about this code, but it looks like
this might be a better fix?
assert(ld == NULL || ld->getSrc(0) != NULL)
I cc'ed Tobias who wrote this code as he'll probably know best.
>// check if we are moving an immediate, propagate it in that case
>if (!ld || ld->fixed || (ld->op != OP_LOAD && ld->op != OP_MOV) ||
> !(ld->src(0).getFile() == FILE_IMMEDIATE))
> bld.mkMovToReg(s, i->getSrc(s));
>else {
> + assert(ld->getSrc(0) != NULL);
> bld.mkMovToReg(s, ld->getSrc(0));
> // Clear the src, to make code elimination possible here before we
> // delete the instruction i later
> --
> 2.21.0
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] nvc0/ir: Fix assert accessing null pointer
Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=111007
Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=67
Signed-off-by: Mark Menzynski
---
src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
b/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
index aca3b0afb1e..1f702a987d8 100644
--- a/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
+++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
@@ -51,12 +51,12 @@ NVC0LegalizeSSA::handleDIV(Instruction *i)
// Generate movs to the input regs for the call we want to generate
for (int s = 0; i->srcExists(s); ++s) {
Instruction *ld = i->getSrc(s)->getInsn();
- assert(ld->getSrc(0) != NULL);
// check if we are moving an immediate, propagate it in that case
if (!ld || ld->fixed || (ld->op != OP_LOAD && ld->op != OP_MOV) ||
!(ld->src(0).getFile() == FILE_IMMEDIATE))
bld.mkMovToReg(s, i->getSrc(s));
else {
+ assert(ld->getSrc(0) != NULL);
bld.mkMovToReg(s, ld->getSrc(0));
// Clear the src, to make code elimination possible here before we
// delete the instruction i later
--
2.21.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
