[v2,38/53] intel/compiler: handle 64-bit to 8-bit conversions

Submitted by Iago Toral Quiroga on Dec. 19, 2018, 11:51 a.m.

Details

Message ID 20181219115121.20815-39-itoral@igalia.com
State New
Headers show
Series "intel: VK_KHR_shader_float16_int8 implementation" ( rev: 3 ) in Mesa

Not browsing as part of any series.

Commit Message

Iago Toral Quiroga Dec. 19, 2018, 11:51 a.m.
These are not directly supported in hardware and brw_nir_lower_conversions
should have taken care of that before we get here.
---
 src/intel/compiler/brw_fs_nir.cpp | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/intel/compiler/brw_fs_nir.cpp b/src/intel/compiler/brw_fs_nir.cpp
index 6089c883c9a..802f5cb0944 100644
--- a/src/intel/compiler/brw_fs_nir.cpp
+++ b/src/intel/compiler/brw_fs_nir.cpp
@@ -818,6 +818,10 @@  fs_visitor::nir_emit_alu(const fs_builder &bld, nir_alu_instr *instr)
    case nir_op_f2f16:
    case nir_op_i2f16:
    case nir_op_u2f16:
+   case nir_op_i2i8:
+   case nir_op_u2u8:
+   case nir_op_f2i8:
+   case nir_op_f2u8:
       assert(type_sz(op[0].type) < 8); /* brw_nir_lower_conversions */
       inst = bld.MOV(result, op[0]);
       inst->saturate = instr->dest.saturate;
@@ -860,8 +864,6 @@  fs_visitor::nir_emit_alu(const fs_builder &bld, nir_alu_instr *instr)
    case nir_op_u2u32:
    case nir_op_i2i16:
    case nir_op_u2u16:
-   case nir_op_i2i8:
-   case nir_op_u2u8:
       inst = bld.MOV(result, op[0]);
       inst->saturate = instr->dest.saturate;
       break;

Comments

On Wed, Dec 19, 2018 at 12:51:06PM +0100, Iago Toral Quiroga wrote:
> These are not directly supported in hardware and brw_nir_lower_conversions
> should have taken care of that before we get here.

It looks that there are two things actually happening here:

1) For int64/uint64 to 8-case the support is already there and this just moves
   the case to a stronger one with an assert.

2) Actually adding support for DF to 8-bit that didn't exist before.

If this is the case (i.e., I'm not missing something), should we adjust the
commit to say that DF to 8-bit support is added and then add a note in the
commit that I64/U64 to 8-bit gets an additional assertion?

> ---
>  src/intel/compiler/brw_fs_nir.cpp | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/src/intel/compiler/brw_fs_nir.cpp b/src/intel/compiler/brw_fs_nir.cpp
> index 6089c883c9a..802f5cb0944 100644
> --- a/src/intel/compiler/brw_fs_nir.cpp
> +++ b/src/intel/compiler/brw_fs_nir.cpp
> @@ -818,6 +818,10 @@ fs_visitor::nir_emit_alu(const fs_builder &bld, nir_alu_instr *instr)
>     case nir_op_f2f16:
>     case nir_op_i2f16:
>     case nir_op_u2f16:
> +   case nir_op_i2i8:
> +   case nir_op_u2u8:
> +   case nir_op_f2i8:
> +   case nir_op_f2u8:
>        assert(type_sz(op[0].type) < 8); /* brw_nir_lower_conversions */
>        inst = bld.MOV(result, op[0]);
>        inst->saturate = instr->dest.saturate;
> @@ -860,8 +864,6 @@ fs_visitor::nir_emit_alu(const fs_builder &bld, nir_alu_instr *instr)
>     case nir_op_u2u32:
>     case nir_op_i2i16:
>     case nir_op_u2u16:
> -   case nir_op_i2i8:
> -   case nir_op_u2u8:
>        inst = bld.MOV(result, op[0]);
>        inst->saturate = instr->dest.saturate;
>        break;
> -- 
> 2.17.1
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
On Wed, 2019-01-02 at 11:59 +0200, Pohjolainen, Topi wrote:
> On Wed, Dec 19, 2018 at 12:51:06PM +0100, Iago Toral Quiroga wrote:
> > These are not directly supported in hardware and
> > brw_nir_lower_conversions
> > should have taken care of that before we get here.
> 
> It looks that there are two things actually happening here:
> 
> 1) For int64/uint64 to 8-case the support is already there and this
> just moves
>    the case to a stronger one with an assert.
> 
> 2) Actually adding support for DF to 8-bit that didn't exist before.
> 
> If this is the case (i.e., I'm not missing something), should we
> adjust the
> commit to say that DF to 8-bit support is added and then add a note
> in the
> commit that I64/U64 to 8-bit gets an additional assertion?

Yes, I'll edit the commit log, thanks.

Iago

> > ---
> >  src/intel/compiler/brw_fs_nir.cpp | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/intel/compiler/brw_fs_nir.cpp
> > b/src/intel/compiler/brw_fs_nir.cpp
> > index 6089c883c9a..802f5cb0944 100644
> > --- a/src/intel/compiler/brw_fs_nir.cpp
> > +++ b/src/intel/compiler/brw_fs_nir.cpp
> > @@ -818,6 +818,10 @@ fs_visitor::nir_emit_alu(const fs_builder
> > &bld, nir_alu_instr *instr)
> >     case nir_op_f2f16:
> >     case nir_op_i2f16:
> >     case nir_op_u2f16:
> > +   case nir_op_i2i8:
> > +   case nir_op_u2u8:
> > +   case nir_op_f2i8:
> > +   case nir_op_f2u8:
> >        assert(type_sz(op[0].type) < 8); /*
> > brw_nir_lower_conversions */
> >        inst = bld.MOV(result, op[0]);
> >        inst->saturate = instr->dest.saturate;
> > @@ -860,8 +864,6 @@ fs_visitor::nir_emit_alu(const fs_builder &bld,
> > nir_alu_instr *instr)
> >     case nir_op_u2u32:
> >     case nir_op_i2i16:
> >     case nir_op_u2u16:
> > -   case nir_op_i2i8:
> > -   case nir_op_u2u8:
> >        inst = bld.MOV(result, op[0]);
> >        inst->saturate = instr->dest.saturate;
> >        break;
> > -- 
> > 2.17.1
> > 
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 
>