[Mesa-dev] [PATCH] gm107/ir: fix sign bit emission for FADD32I

[Ilia Mirkin]

Oh wait, we don't fold it in because it's a SUB, duh. So that bit makes
sense. I'd slightly prefer flipping the neg modifier, but your call.

On Jul 4, 2016 8:17 AM, "Samuel Pitoiset" <samuel.pitoiset at gmail.com> wrote:

>
>
> On 07/04/2016 01:59 PM, Ilia Mirkin wrote:
>
>> That flips the sign of the immediate. Why not flip 0x35, which is an
>> explicit neg modifier? I guess we mess with the immediate in the other
>> emitters, so r-b either way.
>>
>
> Sure, that flips the sign yeah.
>
> I guess we mess up with the neg modifier actually because it is not set
> for that source. I didn't check the other emitters but that test passes on
> GK107 (and probably on GF100/GK110 as well).
>
>
>> As an aside, how did you hit this? Should have gotten folded in...
>>
>
> With piglit. :)
>
>
>>
>> On Jul 4, 2016 7:12 AM, "Samuel Pitoiset" <samuel.pitoiset at gmail.com
>> <mailto:samuel.pitoiset at gmail.com>> wrote:
>>
>>     When emitting OP_SUB, the sign bit for FADD and FADD32I is not
>>     at the same position. It's at position 45 for FADD but 51 for FADD32I.
>>
>>     This fixes the following piglit test:
>>     tests/spec/arb_fragment_program/fdo30337b.shader_test
>>
>>     Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com
>>     <mailto:samuel.pitoiset at gmail.com>>
>>     Cc: <mesa-stable at lists.freedesktop.org
>>     <mailto:mesa-stable at lists.freedesktop.org>>
>>     ---
>>      src/gallium/drivers/nouveau/codegen/nv50_ir_emit_gm107.cpp | 9
>>     ++++++---
>>      1 file changed, 6 insertions(+), 3 deletions(-)
>>
>>     diff --git
>>     a/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_gm107.cpp
>>     b/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_gm107.cpp
>>     index 2c5e8f6..f1ba27a 100644
>>     --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_gm107.cpp
>>     +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_gm107.cpp
>>     @@ -1234,6 +1234,9 @@ CodeEmitterGM107::emitFADD()
>>            emitABS(0x2e, insn->src(0));
>>            emitNEG(0x2d, insn->src(1));
>>            emitFMZ(0x2c, 1);
>>     +
>>     +      if (insn->op == OP_SUB)
>>     +         code[1] ^= 0x00002000;
>>         } else {
>>            emitInsn(0x08000000);
>>            emitABS(0x39, insn->src(1));
>>     @@ -1243,10 +1246,10 @@ CodeEmitterGM107::emitFADD()
>>            emitNEG(0x35, insn->src(1));
>>            emitCC  (0x34);
>>            emitIMMD(0x14, 32, insn->src(1));
>>     -   }
>>
>>     -   if (insn->op == OP_SUB)
>>     -      code[1] ^= 0x00002000;
>>     +      if (insn->op == OP_SUB)
>>     +         code[1] ^= 0x00080000;
>>     +   }
>>
>>         emitGPR(0x08, insn->src(0));
>>         emitGPR(0x00, insn->def(0));
>>     --
>>     2.8.0
>>
>>     _______________________________________________
>>     mesa-dev mailing list
>>     mesa-dev at lists.freedesktop.org <mailto:mesa-dev at lists.freedesktop.org
>> >
>>     https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>>
> --
> -Samuel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160704/daf8eb96/attachment.html>