Am 19.07.19 um 18:18 schrieb Ilia Mirkin:
On Fri, Jul 19, 2019 at 12:07 PM Tobias Klausmann
<tobias.johannes.klausm...@mni.thm.de> 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 <mmenz...@redhat.com>
---
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<tobias.klausm...@freenet.de>
(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
