On Fri, Oct 24, 2014 at 11:12 AM, Matt Turner <matts...@gmail.com> wrote:
> On Fri, Oct 24, 2014 at 10:47 AM, Francisco Jerez <curroje...@riseup.net>
> wrote:
>> Matt Turner <matts...@gmail.com> writes:
>>
>>> On Fri, Oct 24, 2014 at 1:34 AM, Francisco Jerez <curroje...@riseup.net>
>>> wrote:
>>>> Matt Turner <matts...@gmail.com> writes:
>>>>
>>>>> ---
>>>>> src/mesa/drivers/dri/i965/brw_fs.cpp | 20 ++++++++++++++++++++
>>>>> src/mesa/drivers/dri/i965/brw_fs.h | 1 +
>>>>> src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 3 +++
>>>>> 3 files changed, 24 insertions(+)
>>>>>
>>>>> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
>>>>> b/src/mesa/drivers/dri/i965/brw_fs.cpp
>>>>> index 983e8db..ffe0a5b 100644
>>>>> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
>>>>> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
>>>>> @@ -44,6 +44,7 @@ extern "C" {
>>>>> #include "brw_context.h"
>>>>> #include "brw_eu.h"
>>>>> #include "brw_wm.h"
>>>>> +#include "brw_packed_float.h"
>>>>> }
>>>>> #include "brw_fs.h"
>>>>> #include "brw_cfg.h"
>>>>> @@ -581,6 +582,18 @@ fs_reg::fs_reg(uint32_t u)
>>>>> this->width = 1;
>>>>> }
>>>>>
>>>>> +/** Vector float immediate value constructor. */
>>>>> +fs_reg::fs_reg(uint8_t vf[4])
>>>>> +{
>>>>> + init();
>>>>> + this->file = IMM;
>>>>> + this->type = BRW_REGISTER_TYPE_VF;
>>>>> + this->fixed_hw_reg.dw1.ud = (vf[0] << 0) |
>>>>> + (vf[1] << 8) |
>>>>> + (vf[2] << 16) |
>>>>> + (vf[3] << 24);
>>>>> +}
>>>>> +
>>>>
>>>> IMHO it would be a lot more convenient to have this function take four
>>>> float arguments and carry out the conversion to 8-bit here. I suspect
>>>> that in most cases the arguments of this constructor (and the same goes
>>>> for src_reg) will be known at compile time so we could just add an
>>>> assertion to check that the conversion was carried out exactly.
>>>
>>> That would work if we're constructing VF immediates from known good
>>> values, but in cases like in an optimization, you're always going to
>>> have to check whether the values are representable in VF. If they're
>>> not, the pass will have to fail.
>>>
>>
>> Yeah, but isn't this interface unnecessarily annoying to use in the
>> cases that fit the "known good values" category? (all uses in your
>> series and in my previous code that makes use of VF) -- It led you write
>> code like this:
>>
>> | + uint8_t vf[4] = {0x0, 0x60, 0x70, 0x78};
>> |[...]
>> | + emit(MOV(shift, src_reg(vf)));
>>
>> ...which is pretty hard to decode. Don't you find this much easier to
>> understand? (even if it means we may have to introduce a second
>> constructor at some point in the future)
>>
>> | emit(MOV(shift, src_reg(0, 8, 16, 24)));
Another question, if I do this would you have me make
brw_float_to_vf() a static inline function so that the arguments can
be converted at compile time?
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
