Re: [Mesa-dev] [PATCH] gallium/r600: Replace ALIGN_DIVUP with DIV_ROUND_UP

2016-01-06 Thread Krzysztof A. Sobiecki 
Nicolai Hähnle  writes:

> On 30.12.2015 13:44, Krzysztof A. Sobiecki wrote:
>> Nicolai Hähnle  writes:
>>
>>> On 30.12.2015 08:42, Krzysztof A. Sobiecki wrote:
 Nicolai Hähnle  writes:

> On 29.12.2015 14:27, Krzysztof A. Sobiecki wrote:
>> From: Krzysztof Sobiecki 
>>
>> ALIGN_DIVUP is a driver specific(r600g) macro that duplicates 
>> DIV_ROUND_UP functionality.
>> Replacing it with DIV_ROUND_UP eliminates this problems.
>
> Those macros are actually slightly different, and the assembly
> generated by the ALIGN_DIVUP looks clearly better to me.
>
> I remember seeing a very long thread about this not so long ago - what
> was the resolution there?
>
> Cheers,
> Nicolai
>
 I would like to remove ALIGN_DIVUP first and then debate with
 implementation DIV_ROUND_UP should use.

 btw. I prefer 1 + ((x - 1) / y)
>>>
>>> That produces an incorrect result when x is an unsigned type and equal
>>> to 0 -- and that is something that existing code definitely relies on.
>>>
>>> Cheers,
>>> Nicolai
>>>
>> Then what about (x / y) + (i % y != 0)
>
> Generates similar assembly to the DIV_ROUND_UP version.
>
> Anyway, now that I look at it again I'd say just go ahead and add my
> R-b. Yes, the assembly looks slightly worse, but only slightly, and
> avoiding surprises with overflows down the line seems like a good
> idea.
>
> Cheers,
> Nicolai
>
I don't have commit access, can you push it, sorry.
-- 
X was an interactive protocol: 
alpha blending a full-screen image looked like slugs racing down the monitor. 
http://www.keithp.com/~keithp/talks/usenix2000/render.html
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] gallium/r600: Replace ALIGN_DIVUP with DIV_ROUND_UP

2016-01-06 Thread Nicolai Hähnle 

Pushed.

On 06.01.2016 12:10, Krzysztof A. Sobiecki wrote:

Nicolai Hähnle  writes:


On 30.12.2015 13:44, Krzysztof A. Sobiecki wrote:

Nicolai Hähnle  writes:


On 30.12.2015 08:42, Krzysztof A. Sobiecki wrote:

Nicolai Hähnle  writes:


On 29.12.2015 14:27, Krzysztof A. Sobiecki wrote:

From: Krzysztof Sobiecki 

ALIGN_DIVUP is a driver specific(r600g) macro that duplicates DIV_ROUND_UP 
functionality.
Replacing it with DIV_ROUND_UP eliminates this problems.


Those macros are actually slightly different, and the assembly
generated by the ALIGN_DIVUP looks clearly better to me.

I remember seeing a very long thread about this not so long ago - what
was the resolution there?

Cheers,
Nicolai


I would like to remove ALIGN_DIVUP first and then debate with
implementation DIV_ROUND_UP should use.

btw. I prefer 1 + ((x - 1) / y)


That produces an incorrect result when x is an unsigned type and equal
to 0 -- and that is something that existing code definitely relies on.

Cheers,
Nicolai


Then what about (x / y) + (i % y != 0)


Generates similar assembly to the DIV_ROUND_UP version.

Anyway, now that I look at it again I'd say just go ahead and add my
R-b. Yes, the assembly looks slightly worse, but only slightly, and
avoiding surprises with overflows down the line seems like a good
idea.

Cheers,
Nicolai


I don't have commit access, can you push it, sorry.


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

Re: [Mesa-dev] [PATCH] gallium/r600: Replace ALIGN_DIVUP with DIV_ROUND_UP

2015-12-30 Thread Nicolai Hähnle 

On 30.12.2015 13:44, Krzysztof A. Sobiecki wrote:

Nicolai Hähnle  writes:


On 30.12.2015 08:42, Krzysztof A. Sobiecki wrote:

Nicolai Hähnle  writes:


On 29.12.2015 14:27, Krzysztof A. Sobiecki wrote:

From: Krzysztof Sobiecki 

ALIGN_DIVUP is a driver specific(r600g) macro that duplicates DIV_ROUND_UP 
functionality.
Replacing it with DIV_ROUND_UP eliminates this problems.


Those macros are actually slightly different, and the assembly
generated by the ALIGN_DIVUP looks clearly better to me.

I remember seeing a very long thread about this not so long ago - what
was the resolution there?

Cheers,
Nicolai


I would like to remove ALIGN_DIVUP first and then debate with
implementation DIV_ROUND_UP should use.

btw. I prefer 1 + ((x - 1) / y)


That produces an incorrect result when x is an unsigned type and equal
to 0 -- and that is something that existing code definitely relies on.

Cheers,
Nicolai


Then what about (x / y) + (i % y != 0)


Generates similar assembly to the DIV_ROUND_UP version.

Anyway, now that I look at it again I'd say just go ahead and add my 
R-b. Yes, the assembly looks slightly worse, but only slightly, and 
avoiding surprises with overflows down the line seems like a good idea.


Cheers,
Nicolai
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] gallium/r600: Replace ALIGN_DIVUP with DIV_ROUND_UP

2015-12-30 Thread Nicolai Hähnle 

On 30.12.2015 08:42, Krzysztof A. Sobiecki wrote:

Nicolai Hähnle  writes:


On 29.12.2015 14:27, Krzysztof A. Sobiecki wrote:

From: Krzysztof Sobiecki 

ALIGN_DIVUP is a driver specific(r600g) macro that duplicates DIV_ROUND_UP 
functionality.
Replacing it with DIV_ROUND_UP eliminates this problems.


Those macros are actually slightly different, and the assembly
generated by the ALIGN_DIVUP looks clearly better to me.

I remember seeing a very long thread about this not so long ago - what
was the resolution there?

Cheers,
Nicolai


I would like to remove ALIGN_DIVUP first and then debate with
implementation DIV_ROUND_UP should use.

btw. I prefer 1 + ((x - 1) / y)


That produces an incorrect result when x is an unsigned type and equal 
to 0 -- and that is something that existing code definitely relies on.


Cheers,
Nicolai
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] gallium/r600: Replace ALIGN_DIVUP with DIV_ROUND_UP

2015-12-30 Thread Krzysztof A. Sobiecki 
Nicolai Hähnle  writes:

> On 30.12.2015 08:42, Krzysztof A. Sobiecki wrote:
>> Nicolai Hähnle  writes:
>>
>>> On 29.12.2015 14:27, Krzysztof A. Sobiecki wrote:
 From: Krzysztof Sobiecki 

 ALIGN_DIVUP is a driver specific(r600g) macro that duplicates DIV_ROUND_UP 
 functionality.
 Replacing it with DIV_ROUND_UP eliminates this problems.
>>>
>>> Those macros are actually slightly different, and the assembly
>>> generated by the ALIGN_DIVUP looks clearly better to me.
>>>
>>> I remember seeing a very long thread about this not so long ago - what
>>> was the resolution there?
>>>
>>> Cheers,
>>> Nicolai
>>>
>> I would like to remove ALIGN_DIVUP first and then debate with
>> implementation DIV_ROUND_UP should use.
>>
>> btw. I prefer 1 + ((x - 1) / y)
>
> That produces an incorrect result when x is an unsigned type and equal
> to 0 -- and that is something that existing code definitely relies on.
>
> Cheers,
> Nicolai
>
Then what about (x / y) + (i % y != 0)

-- 
X was an interactive protocol: 
alpha blending a full-screen image looked like slugs racing down the monitor. 
http://www.keithp.com/~keithp/talks/usenix2000/render.html
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] gallium/r600: Replace ALIGN_DIVUP with DIV_ROUND_UP

2015-12-30 Thread Krzysztof A. Sobiecki 
Nicolai Hähnle  writes:

> On 29.12.2015 14:27, Krzysztof A. Sobiecki wrote:
>> From: Krzysztof Sobiecki 
>>
>> ALIGN_DIVUP is a driver specific(r600g) macro that duplicates DIV_ROUND_UP 
>> functionality.
>> Replacing it with DIV_ROUND_UP eliminates this problems.
>
> Those macros are actually slightly different, and the assembly
> generated by the ALIGN_DIVUP looks clearly better to me.
>
> I remember seeing a very long thread about this not so long ago - what
> was the resolution there?
>
> Cheers,
> Nicolai
>
I would like to remove ALIGN_DIVUP first and then debate with
implementation DIV_ROUND_UP should use.

btw. I prefer 1 + ((x - 1) / y)

-- 
X was an interactive protocol: 
alpha blending a full-screen image looked like slugs racing down the monitor. 
http://www.keithp.com/~keithp/talks/usenix2000/render.html
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] gallium/r600: Replace ALIGN_DIVUP with DIV_ROUND_UP

2015-12-29 Thread Nicolai Hähnle 

On 29.12.2015 14:27, Krzysztof A. Sobiecki wrote:

From: Krzysztof Sobiecki 

ALIGN_DIVUP is a driver specific(r600g) macro that duplicates DIV_ROUND_UP 
functionality.
Replacing it with DIV_ROUND_UP eliminates this problems.


Those macros are actually slightly different, and the assembly generated 
by the ALIGN_DIVUP looks clearly better to me.


I remember seeing a very long thread about this not so long ago - what 
was the resolution there?


Cheers,
Nicolai



Signed-off-by: Krzysztof A. Sobiecki 
---
  src/gallium/drivers/r600/evergreen_state.c | 2 +-
  src/gallium/drivers/r600/r600_pipe.h   | 1 -
  src/gallium/drivers/r600/r600_state.c  | 2 +-
  3 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/src/gallium/drivers/r600/evergreen_state.c 
b/src/gallium/drivers/r600/evergreen_state.c
index 1aee7dd..9dfb849 100644
--- a/src/gallium/drivers/r600/evergreen_state.c
+++ b/src/gallium/drivers/r600/evergreen_state.c
@@ -1956,7 +1956,7 @@ static void evergreen_emit_constant_buffers(struct 
r600_context *rctx,

if (!gs_ring_buffer) {
radeon_set_context_reg_flag(cs, reg_alu_constbuf_size + 
buffer_index * 4,
-   
ALIGN_DIVUP(cb->buffer_size, 256), pkt_flags);
+   
DIV_ROUND_UP(cb->buffer_size, 256), pkt_flags);
radeon_set_context_reg_flag(cs, reg_alu_const_cache + 
buffer_index * 4, va >> 8,
pkt_flags);
}
diff --git a/src/gallium/drivers/r600/r600_pipe.h 
b/src/gallium/drivers/r600/r600_pipe.h
index 31f2a72..0e4dd16 100644
--- a/src/gallium/drivers/r600/r600_pipe.h
+++ b/src/gallium/drivers/r600/r600_pipe.h
@@ -946,7 +946,6 @@ static inline uint32_t S_FIXED(float value, uint32_t 
frac_bits)
  {
return value * (1 << frac_bits);
  }
-#define ALIGN_DIVUP(x, y) (((x) + (y) - 1) / (y))

  /* 12.4 fixed-point */
  static inline unsigned r600_pack_float_12p4(float x)
diff --git a/src/gallium/drivers/r600/r600_state.c 
b/src/gallium/drivers/r600/r600_state.c
index 43b8074..f60e304 100644
--- a/src/gallium/drivers/r600/r600_state.c
+++ b/src/gallium/drivers/r600/r600_state.c
@@ -1768,7 +1768,7 @@ static void r600_emit_constant_buffers(struct 
r600_context *rctx,

if (!gs_ring_buffer) {
radeon_set_context_reg(cs, reg_alu_constbuf_size + 
buffer_index * 4,
-  ALIGN_DIVUP(cb->buffer_size, 
256));
+  DIV_ROUND_UP(cb->buffer_size, 
256));
radeon_set_context_reg(cs, reg_alu_const_cache + buffer_index 
* 4, offset >> 8);
}



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

[Mesa-dev] [PATCH] gallium/r600: Replace ALIGN_DIVUP with DIV_ROUND_UP

2015-12-29 Thread Krzysztof A. Sobiecki 
From: Krzysztof Sobiecki 

ALIGN_DIVUP is a driver specific(r600g) macro that duplicates DIV_ROUND_UP 
functionality.
Replacing it with DIV_ROUND_UP eliminates this problems.

Signed-off-by: Krzysztof A. Sobiecki 
---
 src/gallium/drivers/r600/evergreen_state.c | 2 +-
 src/gallium/drivers/r600/r600_pipe.h   | 1 -
 src/gallium/drivers/r600/r600_state.c  | 2 +-
 3 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/src/gallium/drivers/r600/evergreen_state.c 
b/src/gallium/drivers/r600/evergreen_state.c
index 1aee7dd..9dfb849 100644
--- a/src/gallium/drivers/r600/evergreen_state.c
+++ b/src/gallium/drivers/r600/evergreen_state.c
@@ -1956,7 +1956,7 @@ static void evergreen_emit_constant_buffers(struct 
r600_context *rctx,
 
if (!gs_ring_buffer) {
radeon_set_context_reg_flag(cs, reg_alu_constbuf_size + 
buffer_index * 4,
-   
ALIGN_DIVUP(cb->buffer_size, 256), pkt_flags);
+   
DIV_ROUND_UP(cb->buffer_size, 256), pkt_flags);
radeon_set_context_reg_flag(cs, reg_alu_const_cache + 
buffer_index * 4, va >> 8,
pkt_flags);
}
diff --git a/src/gallium/drivers/r600/r600_pipe.h 
b/src/gallium/drivers/r600/r600_pipe.h
index 31f2a72..0e4dd16 100644
--- a/src/gallium/drivers/r600/r600_pipe.h
+++ b/src/gallium/drivers/r600/r600_pipe.h
@@ -946,7 +946,6 @@ static inline uint32_t S_FIXED(float value, uint32_t 
frac_bits)
 {
return value * (1 << frac_bits);
 }
-#define ALIGN_DIVUP(x, y) (((x) + (y) - 1) / (y))
 
 /* 12.4 fixed-point */
 static inline unsigned r600_pack_float_12p4(float x)
diff --git a/src/gallium/drivers/r600/r600_state.c 
b/src/gallium/drivers/r600/r600_state.c
index 43b8074..f60e304 100644
--- a/src/gallium/drivers/r600/r600_state.c
+++ b/src/gallium/drivers/r600/r600_state.c
@@ -1768,7 +1768,7 @@ static void r600_emit_constant_buffers(struct 
r600_context *rctx,
 
if (!gs_ring_buffer) {
radeon_set_context_reg(cs, reg_alu_constbuf_size + 
buffer_index * 4,
-  ALIGN_DIVUP(cb->buffer_size, 
256));
+  DIV_ROUND_UP(cb->buffer_size, 
256));
radeon_set_context_reg(cs, reg_alu_const_cache + 
buffer_index * 4, offset >> 8);
}
 
-- 
2.7.0.rc0.173.g4a846af

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