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 > > <[email protected]> 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=111167 > > > > `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 <[email protected]> > > > > > --- > > > > > 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<[email protected]> > (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 [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
