Re: [Mesa-dev] [PATCH 3/9] glsl: Make is_16bit_constant from i965 an ir_constant method.
On 04/07/2014 06:24 AM, Ian Romanick wrote: On 04/06/2014 11:49 PM, Kenneth Graunke wrote: The i965 MUL instruction doesn't natively support 32-bit by 32-bit integer multiplication; additional instructions (MACH/MOV) are required. However, we can avoid those if we know one of the operands can be represented in 16 bits or less. The vector backend's is_16bit_constant static helper function checks for this. In IVB+ the hardware actually does 32-bit by 16-bit to give a 48-bit result (on SNB and earlier it's 16-bit by 16-bit... yay). According to Emil Persson, new generation consoles have similar (but different) integer multiplication issues (see slide 35 from http://www.humus.name/index.php?page=CommentsID=371). Should we expose a lesser precision integer multiply function similar to mul24()? As, say, a GLSL built-in function? Possibly. Presumably our restrictions are different than anyone else's though, and even ours changed between generations. So I wonder if anyone would bother using what we cooked up. Plus, Broadwell just handles this properly, so this really only saves a couple of instructions on old hardware. Obviously, it's not really old, but...by the time we draft a GL extension, implement it, and get people to bother using it in upcoming applications...we'll be down the road a ways. So, I doubt it's really worth doing, but I wouldn't be horribly against it if someone bothered. One big source of integer multiplication is for computing byte offsets into UBOs or other constant buffers. In that case, we're usually scaling by a small constant factor (say, 4 or 16 bytes), which is well covered by our existing optimization. --Ken signature.asc Description: OpenPGP digital signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/9] glsl: Make is_16bit_constant from i965 an ir_constant method.
On 04/07/2014 05:41 AM, Ilia Mirkin wrote: On Mon, Apr 7, 2014 at 2:49 AM, Kenneth Graunke kenn...@whitecape.org wrote: The i965 MUL instruction doesn't natively support 32-bit by 32-bit integer multiplication; additional instructions (MACH/MOV) are required. However, we can avoid those if we know one of the operands can be represented in 16 bits or less. The vector backend's is_16bit_constant static helper function checks for this. We want to be able to use it in the scalar backend as well, which means moving the function to a more generally-usable location. Since it isn't i965 specific, I decided to make it an ir_constant method, in case it ends up being useful to other people as well. Signed-off-by: Kenneth Graunke kenn...@whitecape.org --- src/glsl/ir.cpp| 9 + src/glsl/ir.h | 11 +++ src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp | 18 ++ 3 files changed, 22 insertions(+), 16 deletions(-) diff --git a/src/glsl/ir.cpp b/src/glsl/ir.cpp index a41eddf..07c0a9b 100644 --- a/src/glsl/ir.cpp +++ b/src/glsl/ir.cpp @@ -1223,6 +1223,15 @@ ir_constant::is_basis() const return ones == 1; } +bool +ir_constant::is_16bit_integer_constant() const Perhaps make this is_uint16_constant? Otherwise one might assume based on the name that it would return true for a value like -1. I like that. I've renamed it to is_uint16_constant and updated the comments to clarify that it can return true for signed values, as long as those values are non-negative and fit in 16-bits. --Ken signature.asc Description: OpenPGP digital signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/9] glsl: Make is_16bit_constant from i965 an ir_constant method.
On Mon, Apr 7, 2014 at 2:49 AM, Kenneth Graunke kenn...@whitecape.org wrote:
The i965 MUL instruction doesn't natively support 32-bit by 32-bit
integer multiplication; additional instructions (MACH/MOV) are required.
However, we can avoid those if we know one of the operands can be
represented in 16 bits or less. The vector backend's is_16bit_constant
static helper function checks for this.
We want to be able to use it in the scalar backend as well, which means
moving the function to a more generally-usable location. Since it isn't
i965 specific, I decided to make it an ir_constant method, in case it
ends up being useful to other people as well.
Signed-off-by: Kenneth Graunke kenn...@whitecape.org
---
src/glsl/ir.cpp| 9 +
src/glsl/ir.h | 11 +++
src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp | 18 ++
3 files changed, 22 insertions(+), 16 deletions(-)
diff --git a/src/glsl/ir.cpp b/src/glsl/ir.cpp
index a41eddf..07c0a9b 100644
--- a/src/glsl/ir.cpp
+++ b/src/glsl/ir.cpp
@@ -1223,6 +1223,15 @@ ir_constant::is_basis() const
return ones == 1;
}
+bool
+ir_constant::is_16bit_integer_constant() const
Perhaps make this is_uint16_constant? Otherwise one might assume based
on the name that it would return true for a value like -1.
+{
+ if (!type-is_integer())
+ return false;
+
+ return value.u[0] (1 16);
+}
+
ir_loop::ir_loop()
{
this-ir_type = ir_type_loop;
diff --git a/src/glsl/ir.h b/src/glsl/ir.h
index ee276d2..862eddc 100644
--- a/src/glsl/ir.h
+++ b/src/glsl/ir.h
@@ -265,6 +265,12 @@ public:
*/
virtual bool is_basis() const;
+ /**
+* Determine if an r-value is an integer constant that fits in 16 bits.
+*
+* \sa ir_constant::is_16bit_integer_constant.
+*/
+ virtual bool is_16bit_integer_constant() const { return false; }
/**
* Return a generic value of error_type.
@@ -2165,6 +2171,11 @@ public:
virtual bool is_basis() const;
/**
+* Return true for integer constants which can be represented in 16 bits.
+*/
+ virtual bool is_16bit_integer_constant() const;
+
+ /**
* Value of the constant.
*
* The field used to back the values supplied by the constant is
determined
diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
index c73e58d..3df1c9b 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
+++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
@@ -1174,20 +1174,6 @@ vec4_visitor::emit_lrp(const dst_reg dst,
}
}
-static bool
-is_16bit_constant(ir_rvalue *rvalue)
-{
- ir_constant *constant = rvalue-as_constant();
- if (!constant)
- return false;
-
- if (constant-type != glsl_type::int_type
- constant-type != glsl_type::uint_type)
- return false;
-
- return constant-value.u[0] (1 16);
-}
-
void
vec4_visitor::visit(ir_expression *ir)
{
@@ -1371,12 +1357,12 @@ vec4_visitor::visit(ir_expression *ir)
* operand. If we can determine that one of the args is in the low
* 16 bits, though, we can just emit a single MUL.
*/
- if (is_16bit_constant(ir-operands[0])) {
+ if (ir-operands[0]-is_16bit_integer_constant()) {
if (brw-gen 7)
emit(MUL(result_dst, op[0], op[1]));
else
emit(MUL(result_dst, op[1], op[0]));
- } else if (is_16bit_constant(ir-operands[1])) {
+ } else if (ir-operands[1]-is_16bit_integer_constant()) {
if (brw-gen 7)
emit(MUL(result_dst, op[1], op[0]));
else
--
1.9.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/9] glsl: Make is_16bit_constant from i965 an ir_constant method.
On 04/06/2014 11:49 PM, Kenneth Graunke wrote:
The i965 MUL instruction doesn't natively support 32-bit by 32-bit
integer multiplication; additional instructions (MACH/MOV) are required.
However, we can avoid those if we know one of the operands can be
represented in 16 bits or less. The vector backend's is_16bit_constant
static helper function checks for this.
In IVB+ the hardware actually does 32-bit by 16-bit to give a 48-bit
result (on SNB and earlier it's 16-bit by 16-bit... yay). According to
Emil Persson, new generation consoles have similar (but different)
integer multiplication issues (see slide 35 from
http://www.humus.name/index.php?page=CommentsID=371). Should we expose
a lesser precision integer multiply function similar to mul24()?
We want to be able to use it in the scalar backend as well, which means
moving the function to a more generally-usable location. Since it isn't
i965 specific, I decided to make it an ir_constant method, in case it
ends up being useful to other people as well.
Signed-off-by: Kenneth Graunke kenn...@whitecape.org
---
src/glsl/ir.cpp| 9 +
src/glsl/ir.h | 11 +++
src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp | 18 ++
3 files changed, 22 insertions(+), 16 deletions(-)
diff --git a/src/glsl/ir.cpp b/src/glsl/ir.cpp
index a41eddf..07c0a9b 100644
--- a/src/glsl/ir.cpp
+++ b/src/glsl/ir.cpp
@@ -1223,6 +1223,15 @@ ir_constant::is_basis() const
return ones == 1;
}
+bool
+ir_constant::is_16bit_integer_constant() const
+{
+ if (!type-is_integer())
+ return false;
+
+ return value.u[0] (1 16);
+}
+
ir_loop::ir_loop()
{
this-ir_type = ir_type_loop;
diff --git a/src/glsl/ir.h b/src/glsl/ir.h
index ee276d2..862eddc 100644
--- a/src/glsl/ir.h
+++ b/src/glsl/ir.h
@@ -265,6 +265,12 @@ public:
*/
virtual bool is_basis() const;
+ /**
+* Determine if an r-value is an integer constant that fits in 16 bits.
+*
+* \sa ir_constant::is_16bit_integer_constant.
+*/
+ virtual bool is_16bit_integer_constant() const { return false; }
/**
* Return a generic value of error_type.
@@ -2165,6 +2171,11 @@ public:
virtual bool is_basis() const;
/**
+* Return true for integer constants which can be represented in 16 bits.
+*/
+ virtual bool is_16bit_integer_constant() const;
+
+ /**
* Value of the constant.
*
* The field used to back the values supplied by the constant is
determined
diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
index c73e58d..3df1c9b 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
+++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
@@ -1174,20 +1174,6 @@ vec4_visitor::emit_lrp(const dst_reg dst,
}
}
-static bool
-is_16bit_constant(ir_rvalue *rvalue)
-{
- ir_constant *constant = rvalue-as_constant();
- if (!constant)
- return false;
-
- if (constant-type != glsl_type::int_type
- constant-type != glsl_type::uint_type)
- return false;
-
- return constant-value.u[0] (1 16);
-}
-
void
vec4_visitor::visit(ir_expression *ir)
{
@@ -1371,12 +1357,12 @@ vec4_visitor::visit(ir_expression *ir)
* operand. If we can determine that one of the args is in the low
* 16 bits, though, we can just emit a single MUL.
*/
- if (is_16bit_constant(ir-operands[0])) {
+ if (ir-operands[0]-is_16bit_integer_constant()) {
if (brw-gen 7)
emit(MUL(result_dst, op[0], op[1]));
else
emit(MUL(result_dst, op[1], op[0]));
- } else if (is_16bit_constant(ir-operands[1])) {
+ } else if (ir-operands[1]-is_16bit_integer_constant()) {
if (brw-gen 7)
emit(MUL(result_dst, op[1], op[0]));
else
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
