[Nouveau] [PATCH] nv50/ir: Handle OP_CVT when folding constant expressions

[Tobias Klausmann]

On 11.01.2015 23:53, Ilia Mirkin wrote:
> On Sun, Jan 11, 2015 at 5:48 PM, Tobias Klausmann 
> <tobias.johannes.klausmann at mni.thm.de> wrote:
>> On 11.01.2015 23:12, Ilia Mirkin wrote:
>>> On Sun, Jan 11, 2015 at 5:08 PM, Tobias Klausmann 
>>> <tobias.johannes.klausmann at mni.thm.de> wrote:
>>>> On 11.01.2015 22:54, Ilia Mirkin wrote:
>>>>> On Sun, Jan 11, 2015 at 4:40 PM, Tobias Klausmann 
>>>>> <tobias.johannes.klausmann at mni.thm.de> wrote:
>>>>>> Folding for conversions: F32->(U{16/32}, S{16/32}) and (U{16/32}, 
>>>>>> {S16/32})->F32 Signed-off-by: Tobias Klausmann 
>>>>>> <tobias.johannes.klausmann at mni.thm.de> --- 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

Haven't found a piglit test triggering that, but i have created a TGSI 
shader on my own. That's the only reason i am writing this email and not 
just posting the patch. This one is collapsed just fine though.

FRAG
DCL OUT[0..2], COLOR
DCL CONST[0..2]
DCL TEMP[0..2], LOCAL
IMM[0] FLT32 { 1, 2, 3, 4}
IMM[1] UINT32 { 5, 6, 7, 8}
0: TEX TEMP[0], IMM[0], SAMP[0], 2D_ARRAY
1: TXF TEMP[1], IMM[1], SAMP[0], 2D_ARRAY
2: MOV OUT[0], TEMP[0]
3: MOV OUT[1], TEMP[1]
4: END


Greetings,
Tobias