Discussion:
[Mesa-dev] [PATCH 1/2] nvc0/ir: always use limm form for log ops with immediates
Karol Herbst
2018-03-16 22:05:56 UTC
Permalink
in the short imm form 0x80000 was sign-extended to 0xfff80000 which leads to
wrong results.

Signed-off-by: Karol Herbst <***@redhat.com>
---
src/gallium/drivers/nouveau/codegen/nv50_ir_emit_gk110.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_gk110.cpp b/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_gk110.cpp
index 370427d0d13..a988f169bd4 100644
--- a/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_gk110.cpp
+++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_gk110.cpp
@@ -877,7 +877,7 @@ CodeEmitterGK110::emitLogicOp(const Instruction *i, uint8_t subOp)
code[1] |= 7 << 10;
}
} else
- if (isLIMM(i->src(1), TYPE_S32)) {
+ if (i->src(1).getFile() == FILE_IMMEDIATE) {
emitForm_L(i, 0x200, 0, i->src(1).mod);
code[1] |= subOp << 24;
NOT_(3a, 0);
--
2.14.3
Karol Herbst
2018-03-16 22:05:57 UTC
Permalink
in the short imm form 0x80000 was sign-extended to 0xfff80000 which leads to
wrong results.

Signed-off-by: Karol Herbst <***@redhat.com>
---
src/gallium/drivers/nouveau/codegen/nv50_ir_emit_nvc0.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_nvc0.cpp b/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_nvc0.cpp
index 58594f02c7f..644bd11ec28 100644
--- a/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_nvc0.cpp
+++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_nvc0.cpp
@@ -887,7 +887,7 @@ CodeEmitterNVC0::emitLogicOp(const Instruction *i, uint8_t subOp)
}
} else
if (i->encSize == 8) {
- if (isLIMM(i->src(1), TYPE_U32)) {
+ if (i->src(1).getFile() == FILE_IMMEDIATE) {
emitForm_A(i, HEX64(38000000, 00000002));

if (i->flagsDef >= 0)
--
2.14.3
Ilia Mirkin
2018-03-16 22:10:51 UTC
Permalink
You got the subjects backwards.
Post by Karol Herbst
in the short imm form 0x80000 was sign-extended to 0xfff80000 which leads to
wrong results.
---
src/gallium/drivers/nouveau/codegen/nv50_ir_emit_nvc0.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_nvc0.cpp b/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_nvc0.cpp
index 58594f02c7f..644bd11ec28 100644
--- a/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_nvc0.cpp
+++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_nvc0.cpp
@@ -887,7 +887,7 @@ CodeEmitterNVC0::emitLogicOp(const Instruction *i, uint8_t subOp)
}
} else
if (i->encSize == 8) {
- if (isLIMM(i->src(1), TYPE_U32)) {
+ if (i->src(1).getFile() == FILE_IMMEDIATE) {
emitForm_A(i, HEX64(38000000, 00000002));
if (i->flagsDef >= 0)
Should probably assert i->src(1).mod == 0?
Karol Herbst
2018-03-16 22:22:40 UTC
Permalink
Post by Ilia Mirkin
You got the subjects backwards.
Post by Karol Herbst
in the short imm form 0x80000 was sign-extended to 0xfff80000 which leads to
wrong results.
---
src/gallium/drivers/nouveau/codegen/nv50_ir_emit_nvc0.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_nvc0.cpp b/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_nvc0.cpp
index 58594f02c7f..644bd11ec28 100644
--- a/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_nvc0.cpp
+++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_nvc0.cpp
@@ -887,7 +887,7 @@ CodeEmitterNVC0::emitLogicOp(const Instruction *i, uint8_t subOp)
}
} else
if (i->encSize == 8) {
- if (isLIMM(i->src(1), TYPE_U32)) {
+ if (i->src(1).getFile() == FILE_IMMEDIATE) {
emitForm_A(i, HEX64(38000000, 00000002));
if (i->flagsDef >= 0)
Should probably assert i->src(1).mod == 0?
well maybe we could just do the fancy imm.applyTo() thing or something
as well as we do in the gk110 case. Sounds like a reasonable thing to
do instead putting asserts all over the code.

Loading...