Matt Turner <matts...@gmail.com> writes:
> 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?Not necessarily, we don't really need the arguments to be converted at compile time if we use a dynamic assert.
pgpUZExVKuwg8.pgp
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
