On Sun, Jan 11, 2015 at 5:48 PM, Tobias Klausmann <[email protected]> wrote: > > > On 11.01.2015 23:12, Ilia Mirkin wrote: >> >> On Sun, Jan 11, 2015 at 5:08 PM, Tobias Klausmann >> <[email protected]> wrote: >>> >>> >>> On 11.01.2015 22:54, Ilia Mirkin wrote: >>>> >>>> On Sun, Jan 11, 2015 at 4:40 PM, Tobias Klausmann >>>> <[email protected]> wrote: >>>>> >>>>> Folding for conversions: F32->(U{16/32}, S{16/32}) and (U{16/32}, >>>>> {S16/32})->F32 >>>>> >>>>> Signed-off-by: Tobias Klausmann <[email protected]> >>>>> --- >>>>> V2: Split out F64 parts >>>>> V3: remove handling of saturate for (U/S)32, >>>>> >>>>> .../drivers/nouveau/codegen/nv50_ir_peephole.cpp | 73 >>>>> ++++++++++++++++++++++ >>>>> 1 file changed, 73 insertions(+) >>>>> >>>>> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp >>>>> b/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp >>>>> index 21d20ca..aaf0d0d 100644 >>>>> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp >>>>> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp >>>>> @@ -997,6 +997,79 @@ ConstantFolding::opnd(Instruction *i, >>>>> ImmediateValue >>>>> &imm0, int s) >>>>> i->op = OP_MOV; >>>>> break; >>>>> } >>>>> + case OP_CVT: { >>>>> + Storage res; >>>>> + bld.setPosition(i, true); /* make sure bld is init'ed */ >>>>> + switch(i->dType) { >>>>> + case TYPE_U16: >>>>> + switch (i->sType) { >>>>> + case TYPE_F32: >>>>> + if (i->saturate) >>>>> + res.data.u16 = util_iround(CLAMP(imm0.reg.data.f32, 0, >>>>> + UINT16_MAX)); >>>>> + else >>>>> + res.data.u16 = util_iround(imm0.reg.data.f32); >>>>> + break; >>>>> + default: >>>>> + return; >>>>> + } >>>> >>>> This won't get hit for the U32 -> U16 conversion though right? Did you >>>> test that case? Am I misreading/misunderstanding perhaps? >>> >>> A complete piglit run did not hit i->saturate for U32 or S32. That said, >>> i >>> kept the assert() there on purpose for now to actually make sure we are >>> no >>> hitting such a case. Do i misread you now? :) >> >> From my read of the code, we'd hit that case now with TXF on a >> 2D_ARRAY with a constant as the array element. i.e. a piglit with >> >> uniform sampler2DArray foo; >> texelFetch(foo, ivec3(1, 2, 3)); > > Tested this (hope i did the right thing) and the assert did not get > triggered, but i am still uncertain of this. > -> move the assert into the F32 case for U32/S32 just to make sure... > switch (i->sType) > case TYPE_F32: > assert(...) > ... > > other than that, we are not even going to fold U32 -> U16 ;-)
Right, and that's the problem. Try it with a piglit that has the code I suggest... if you don't end up collapsing it, include the TGSI that's generated (and also the shader test source) and we'll go from there. -ilia _______________________________________________ Nouveau mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/nouveau
