On Wed, Jul 22, 2015 at 12:55 AM, Francisco Jerez <curroje...@riseup.net> wrote:
> Jason Ekstrand <ja...@jlekstrand.net> writes:
>
>> On Sat, Jul 18, 2015 at 7:34 AM, Francisco Jerez <curroje...@riseup.net>
>> wrote:
>>> This should match the set of cases in which we currently call fail()
>>> or no16() from the emit_texture_*() methods and the ones in which
>>> emit_texture_gen4() enables the SIMD16 workaround.
>>>
>>> Hint for reviewers: It's not a big deal if I happen to have missed
>>> some case here, it will just lead to an assertion failure down the
>>> road which is easily fixable, however being stricter than necessary
>>> won't cause any visible breakage, it would just decrease performance
>>> silently due to the unnecessary message splitting, so feel free to
>>> double-check that all cases listed here already cause a SIMD8/16
>>> fall-back with the current texturing code -- You may want to skip over
>>> the Gen5-6 cases though if you don't have pencil and paper at hand.
>>
>> I'll believe you. Even if you got it wrong, the worst that happens is
>> we hit the fail case when we try and lower.
>>
>>> ---
>>> src/mesa/drivers/dri/i965/brw_fs.cpp | 27 +++++++++++++++++++++++++++
>>> 1 file changed, 27 insertions(+)
>>>
>>> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
>>> b/src/mesa/drivers/dri/i965/brw_fs.cpp
>>> index 043d9e9..f291202 100644
>>> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
>>> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
>>> @@ -3918,6 +3918,33 @@ get_lowered_simd_width(const struct brw_device_info
>>> *devinfo,
>>> /* Dual-source FB writes are unsupported in SIMD16 mode. */
>>> return (inst->src[1].file != BAD_FILE ? 8 : inst->exec_size);
>>>
>>> + case SHADER_OPCODE_TXD_LOGICAL:
>>> + /* TXD is unsupported in SIMD16 mode. */
>>> + return 8;
>>> +
>>> + case SHADER_OPCODE_TG4_OFFSET_LOGICAL: {
>>> + /* gather4_po_c is unsupported in SIMD16 mode. */
>>> + const fs_reg &shadow_c = inst->src[1];
>>> + return (shadow_c.file != BAD_FILE ? 8 : inst->exec_size);
>>> + }
>>> + case SHADER_OPCODE_TXL_LOGICAL:
>>> + case FS_OPCODE_TXB_LOGICAL: {
>>> + /* Gen4 doesn't have SIMD8 non-shadow-compare bias/LOD instructions,
>>> and
>>> + * Gen4-6 don't support TXL and TXB with shadow comparison in SIMD16
>>> + * mode.
>>> + */
>>> + const fs_reg &shadow_c = inst->src[1];
>>> + return (devinfo->gen == 4 && shadow_c.file == BAD_FILE ? 16 :
>>> + devinfo->gen < 7 && shadow_c.file != BAD_FILE ? 8 :
>>> + inst->exec_size);
>>
>> Could we please use an if-ladder here. Nested ternaries are hard to
>> read and kind of pointless given that we can return multiple places in
>> the if.
>>
> Heh, I always have the very opposite style itch -- If you can compute
> something succinctly with a handful of "pure" expressions why venture
> into the dangerous realm of statements?
Heh... The mathematician in me is tempted to agree with you. However,
the programmer in me likes his control-flow to have obvious nesting
and indentation. :-)
> I think John Backus' article "Can programming be liberated from the von
> Neumann style?" although slightly outdated elaborates the point pretty
> well.
>
> But sure, if you'd like it better with an if-ladder why not.
>
>>> + }
>>> + case SHADER_OPCODE_TXF_LOGICAL:
>>> + case SHADER_OPCODE_TXS_LOGICAL:
>>> + /* Gen4 doesn't have SIMD8 variants for the RESINFO and LD-with-LOD
>>> + * messages. Use SIMD16 instead.
>>> + */
>>> + return (devinfo->gen == 4 ? 16 : inst->exec_size);
>>
>> I'd kind of also like this ternary to go, but I'm ok with it if you
>> want to keep it.
>>
>>> +
>>> default:
>>> return inst->exec_size;
>>> }
>>> --
>>> 2.4.3
>>>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
