Re: [Mesa-dev] [PATCH v3 12/44] nir: add new fadd, fsub, fmul opcodes taking into account rounding mode

2019-02-07 Thread Samuel Iglesias Gonsálvez 


On 2/6/19 12:47 PM, Connor Abbott wrote:
> 
> 
> On Wed, Feb 6, 2019 at 11:46 AM Samuel Iglesias Gonsálvez
> mailto:sigles...@igalia.com>> wrote:
> 
> According to Vulkan spec, the new execution modes affect only
> correctly rounded SPIR-V instructions, which includes fadd,
> fsub and fmul.
> 
> Signed-off-by: Samuel Iglesias Gonsálvez  >
> ---
>  src/compiler/nir/nir_opcodes.py | 17 +
>  1 file changed, 17 insertions(+)
> 
> diff --git a/src/compiler/nir/nir_opcodes.py
> b/src/compiler/nir/nir_opcodes.py
> index f8997444c28..7b45d38f460 100644
> --- a/src/compiler/nir/nir_opcodes.py
> +++ b/src/compiler/nir/nir_opcodes.py
> @@ -453,9 +453,15 @@ def binop_convert(name, out_type, in_type,
> alg_props, const_expr):
>     opcode(name, 0, out_type, [0, 0], [in_type, in_type],
>            False, alg_props, const_expr, "")
> 
> +def binop_convert_rounding_mode(name, out_type, in_type, alg_props,
> const_expr, rounding_mode):
> +   opcode(name, 0, out_type, [0, 0], [in_type, in_type], False,
> alg_props, const_expr, rounding_mode)
> +
>  def binop(name, ty, alg_props, const_expr):
>     binop_convert(name, ty, ty, alg_props, const_expr)
> 
> +def binop_rounding_mode(name, ty, alg_props, const_expr,
> rounding_mode):
> +   binop_convert_rounding_mode(name, ty, ty, alg_props, const_expr,
> rounding_mode)
> +
>  def binop_compare(name, ty, alg_props, const_expr):
>     binop_convert(name, tbool1, ty, alg_props, const_expr)
> 
> @@ -490,13 +496,24 @@ def binop_reduce(name, output_size,
> output_type, src_type, prereduce_expr,
>            final(reduce_(reduce_(src0, src1), reduce_(src2, src3))), "")
> 
>  binop("fadd", tfloat, commutative + associative, "src0 + src1")
> +binop_rounding_mode("fadd_rtne", tfloat, commutative + associative,
> +                    "_mesa_roundeven(src0 + src1)", "_rtne")
> 
> 
> There are two things wrong here:
> - The default floating-point environment, and the one Mesa expects, does
> round-to-nearest-even so it's rtz that needs special handling.
> - _mesa_roundeven here is a no-op (it'll implicitly expand to a double
> and then convert back to a float). The rounding actually happens as part
> of the addition itself. I'm not sure if adding as double (with
> round-to-nearest-even) and then rounding back to a float will work, but
> that definitely won't work for doubles. I think you'll have to implement
> round-to-zero addition yourself, or fiddle with the CPU's floating-point
> environment.
> 
> The same goes multiply and fma.
>  

In order to avoid flooding the ML with more patches, I created a MR with
the suggested changes:

https://gitlab.freedesktop.org/mesa/mesa/merge_requests/223

Please take a look at them and let me know if now it is fine.

Thanks,

Sam

> 
> +binop_rounding_mode("fadd_rtz", tfloat, commutative + associative,
> "src0 + src1", "_rtz")
> +
>  binop("iadd", tint, commutative + associative, "src0 + src1")
>  binop("uadd_sat", tuint, commutative,
>        "(src0 + src1) < src0 ? UINT64_MAX : (src0 + src1)")
>  binop("fsub", tfloat, "", "src0 - src1")
> +binop_rounding_mode("fsub_rtne", tfloat, "",
> +                    "_mesa_roundeven(src0 - src1)", "_rtne")
> +binop_rounding_mode("fsub_rtz", tfloat, "", "src0 - src1", "_rtz")
>  binop("isub", tint, "", "src0 - src1")
> 
>  binop("fmul", tfloat, commutative + associative, "src0 * src1")
> +binop_rounding_mode("fmul_rtne", tfloat, commutative + associative,
> +                    "_mesa_roundeven(src0 * src1)", "_rtne")
> +binop_rounding_mode("fmul_rtz", tfloat, commutative + associative,
> "src0 * src1", "_rtz")
> +
>  # low 32-bits of signed/unsigned integer multiply
>  binop("imul", tint, commutative + associative, "src0 * src1")
> 
> -- 
> 2.19.1
> 
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org 
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 



signature.asc
Description: OpenPGP digital signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v3 12/44] nir: add new fadd, fsub, fmul opcodes taking into account rounding mode

2019-02-07 Thread Samuel Iglesias Gonsálvez 


On 2/6/19 12:47 PM, Connor Abbott wrote:
> 
> 
> On Wed, Feb 6, 2019 at 11:46 AM Samuel Iglesias Gonsálvez
> mailto:sigles...@igalia.com>> wrote:
> 
> According to Vulkan spec, the new execution modes affect only
> correctly rounded SPIR-V instructions, which includes fadd,
> fsub and fmul.
> 
> Signed-off-by: Samuel Iglesias Gonsálvez  >
> ---
>  src/compiler/nir/nir_opcodes.py | 17 +
>  1 file changed, 17 insertions(+)
> 
> diff --git a/src/compiler/nir/nir_opcodes.py
> b/src/compiler/nir/nir_opcodes.py
> index f8997444c28..7b45d38f460 100644
> --- a/src/compiler/nir/nir_opcodes.py
> +++ b/src/compiler/nir/nir_opcodes.py
> @@ -453,9 +453,15 @@ def binop_convert(name, out_type, in_type,
> alg_props, const_expr):
>     opcode(name, 0, out_type, [0, 0], [in_type, in_type],
>            False, alg_props, const_expr, "")
> 
> +def binop_convert_rounding_mode(name, out_type, in_type, alg_props,
> const_expr, rounding_mode):
> +   opcode(name, 0, out_type, [0, 0], [in_type, in_type], False,
> alg_props, const_expr, rounding_mode)
> +
>  def binop(name, ty, alg_props, const_expr):
>     binop_convert(name, ty, ty, alg_props, const_expr)
> 
> +def binop_rounding_mode(name, ty, alg_props, const_expr,
> rounding_mode):
> +   binop_convert_rounding_mode(name, ty, ty, alg_props, const_expr,
> rounding_mode)
> +
>  def binop_compare(name, ty, alg_props, const_expr):
>     binop_convert(name, tbool1, ty, alg_props, const_expr)
> 
> @@ -490,13 +496,24 @@ def binop_reduce(name, output_size,
> output_type, src_type, prereduce_expr,
>            final(reduce_(reduce_(src0, src1), reduce_(src2, src3))), "")
> 
>  binop("fadd", tfloat, commutative + associative, "src0 + src1")
> +binop_rounding_mode("fadd_rtne", tfloat, commutative + associative,
> +                    "_mesa_roundeven(src0 + src1)", "_rtne")
> 
> 
> There are two things wrong here:
> - The default floating-point environment, and the one Mesa expects, does
> round-to-nearest-even so it's rtz that needs special handling.
> - _mesa_roundeven here is a no-op (it'll implicitly expand to a double
> and then convert back to a float). The rounding actually happens as part
> of the addition itself. I'm not sure if adding as double (with
> round-to-nearest-even) and then rounding back to a float will work, but
> that definitely won't work for doubles. I think you'll have to implement
> round-to-zero addition yourself, or fiddle with the CPU's floating-point
> environment.
> 
> The same goes multiply and fma.
>  

OK, thanks for the feedback. I will implement the needed RTZ ops and
send a new version.

Sam

> 
> +binop_rounding_mode("fadd_rtz", tfloat, commutative + associative,
> "src0 + src1", "_rtz")
> +
>  binop("iadd", tint, commutative + associative, "src0 + src1")
>  binop("uadd_sat", tuint, commutative,
>        "(src0 + src1) < src0 ? UINT64_MAX : (src0 + src1)")
>  binop("fsub", tfloat, "", "src0 - src1")
> +binop_rounding_mode("fsub_rtne", tfloat, "",
> +                    "_mesa_roundeven(src0 - src1)", "_rtne")
> +binop_rounding_mode("fsub_rtz", tfloat, "", "src0 - src1", "_rtz")
>  binop("isub", tint, "", "src0 - src1")
> 
>  binop("fmul", tfloat, commutative + associative, "src0 * src1")
> +binop_rounding_mode("fmul_rtne", tfloat, commutative + associative,
> +                    "_mesa_roundeven(src0 * src1)", "_rtne")
> +binop_rounding_mode("fmul_rtz", tfloat, commutative + associative,
> "src0 * src1", "_rtz")
> +
>  # low 32-bits of signed/unsigned integer multiply
>  binop("imul", tint, commutative + associative, "src0 * src1")
> 
> -- 
> 2.19.1
> 
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org 
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 



signature.asc
Description: OpenPGP digital signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v3 12/44] nir: add new fadd, fsub, fmul opcodes taking into account rounding mode

2019-02-06 Thread Connor Abbott 
On Wed, Feb 6, 2019 at 11:46 AM Samuel Iglesias Gonsálvez <
sigles...@igalia.com> wrote:

> According to Vulkan spec, the new execution modes affect only
> correctly rounded SPIR-V instructions, which includes fadd,
> fsub and fmul.
>
> Signed-off-by: Samuel Iglesias Gonsálvez 
> ---
>  src/compiler/nir/nir_opcodes.py | 17 +
>  1 file changed, 17 insertions(+)
>
> diff --git a/src/compiler/nir/nir_opcodes.py
> b/src/compiler/nir/nir_opcodes.py
> index f8997444c28..7b45d38f460 100644
> --- a/src/compiler/nir/nir_opcodes.py
> +++ b/src/compiler/nir/nir_opcodes.py
> @@ -453,9 +453,15 @@ def binop_convert(name, out_type, in_type, alg_props,
> const_expr):
> opcode(name, 0, out_type, [0, 0], [in_type, in_type],
>False, alg_props, const_expr, "")
>
> +def binop_convert_rounding_mode(name, out_type, in_type, alg_props,
> const_expr, rounding_mode):
> +   opcode(name, 0, out_type, [0, 0], [in_type, in_type], False,
> alg_props, const_expr, rounding_mode)
> +
>  def binop(name, ty, alg_props, const_expr):
> binop_convert(name, ty, ty, alg_props, const_expr)
>
> +def binop_rounding_mode(name, ty, alg_props, const_expr, rounding_mode):
> +   binop_convert_rounding_mode(name, ty, ty, alg_props, const_expr,
> rounding_mode)
> +
>  def binop_compare(name, ty, alg_props, const_expr):
> binop_convert(name, tbool1, ty, alg_props, const_expr)
>
> @@ -490,13 +496,24 @@ def binop_reduce(name, output_size, output_type,
> src_type, prereduce_expr,
>final(reduce_(reduce_(src0, src1), reduce_(src2, src3))), "")
>
>  binop("fadd", tfloat, commutative + associative, "src0 + src1")
> +binop_rounding_mode("fadd_rtne", tfloat, commutative + associative,
> +"_mesa_roundeven(src0 + src1)", "_rtne")
>

There are two things wrong here:
- The default floating-point environment, and the one Mesa expects, does
round-to-nearest-even so it's rtz that needs special handling.
- _mesa_roundeven here is a no-op (it'll implicitly expand to a double and
then convert back to a float). The rounding actually happens as part of the
addition itself. I'm not sure if adding as double (with
round-to-nearest-even) and then rounding back to a float will work, but
that definitely won't work for doubles. I think you'll have to implement
round-to-zero addition yourself, or fiddle with the CPU's floating-point
environment.

The same goes multiply and fma.


> +binop_rounding_mode("fadd_rtz", tfloat, commutative + associative, "src0
> + src1", "_rtz")
> +
>  binop("iadd", tint, commutative + associative, "src0 + src1")
>  binop("uadd_sat", tuint, commutative,
>"(src0 + src1) < src0 ? UINT64_MAX : (src0 + src1)")
>  binop("fsub", tfloat, "", "src0 - src1")
> +binop_rounding_mode("fsub_rtne", tfloat, "",
> +"_mesa_roundeven(src0 - src1)", "_rtne")
> +binop_rounding_mode("fsub_rtz", tfloat, "", "src0 - src1", "_rtz")
>  binop("isub", tint, "", "src0 - src1")
>
>  binop("fmul", tfloat, commutative + associative, "src0 * src1")
> +binop_rounding_mode("fmul_rtne", tfloat, commutative + associative,
> +"_mesa_roundeven(src0 * src1)", "_rtne")
> +binop_rounding_mode("fmul_rtz", tfloat, commutative + associative, "src0
> * src1", "_rtz")
> +
>  # low 32-bits of signed/unsigned integer multiply
>  binop("imul", tint, commutative + associative, "src0 * src1")
>
> --
> 2.19.1
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH v3 12/44] nir: add new fadd, fsub, fmul opcodes taking into account rounding mode

2019-02-06 Thread Samuel Iglesias Gonsálvez 
According to Vulkan spec, the new execution modes affect only
correctly rounded SPIR-V instructions, which includes fadd,
fsub and fmul.

Signed-off-by: Samuel Iglesias Gonsálvez 
---
 src/compiler/nir/nir_opcodes.py | 17 +
 1 file changed, 17 insertions(+)

diff --git a/src/compiler/nir/nir_opcodes.py b/src/compiler/nir/nir_opcodes.py
index f8997444c28..7b45d38f460 100644
--- a/src/compiler/nir/nir_opcodes.py
+++ b/src/compiler/nir/nir_opcodes.py
@@ -453,9 +453,15 @@ def binop_convert(name, out_type, in_type, alg_props, 
const_expr):
opcode(name, 0, out_type, [0, 0], [in_type, in_type],
   False, alg_props, const_expr, "")
 
+def binop_convert_rounding_mode(name, out_type, in_type, alg_props, 
const_expr, rounding_mode):
+   opcode(name, 0, out_type, [0, 0], [in_type, in_type], False, alg_props, 
const_expr, rounding_mode)
+
 def binop(name, ty, alg_props, const_expr):
binop_convert(name, ty, ty, alg_props, const_expr)
 
+def binop_rounding_mode(name, ty, alg_props, const_expr, rounding_mode):
+   binop_convert_rounding_mode(name, ty, ty, alg_props, const_expr, 
rounding_mode)
+
 def binop_compare(name, ty, alg_props, const_expr):
binop_convert(name, tbool1, ty, alg_props, const_expr)
 
@@ -490,13 +496,24 @@ def binop_reduce(name, output_size, output_type, 
src_type, prereduce_expr,
   final(reduce_(reduce_(src0, src1), reduce_(src2, src3))), "")
 
 binop("fadd", tfloat, commutative + associative, "src0 + src1")
+binop_rounding_mode("fadd_rtne", tfloat, commutative + associative,
+"_mesa_roundeven(src0 + src1)", "_rtne")
+binop_rounding_mode("fadd_rtz", tfloat, commutative + associative, "src0 + 
src1", "_rtz")
+
 binop("iadd", tint, commutative + associative, "src0 + src1")
 binop("uadd_sat", tuint, commutative,
   "(src0 + src1) < src0 ? UINT64_MAX : (src0 + src1)")
 binop("fsub", tfloat, "", "src0 - src1")
+binop_rounding_mode("fsub_rtne", tfloat, "",
+"_mesa_roundeven(src0 - src1)", "_rtne")
+binop_rounding_mode("fsub_rtz", tfloat, "", "src0 - src1", "_rtz")
 binop("isub", tint, "", "src0 - src1")
 
 binop("fmul", tfloat, commutative + associative, "src0 * src1")
+binop_rounding_mode("fmul_rtne", tfloat, commutative + associative,
+"_mesa_roundeven(src0 * src1)", "_rtne")
+binop_rounding_mode("fmul_rtz", tfloat, commutative + associative, "src0 * 
src1", "_rtz")
+
 # low 32-bits of signed/unsigned integer multiply
 binop("imul", tint, commutative + associative, "src0 * src1")
 
-- 
2.19.1

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev