On Friday, October 24, 2014 08:47:03 PM Francisco Jerez 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)));It might not be a bad idea to have both. I agree with Curro - being able to say src_reg(0, 8, 16, 24) is nice and easy to read. But being able to pass in pre-converted values is not a bad idea either, and makes Matt's case work. --Ken
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
