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