[Mesa-dev] [PATCH] nouveau: use bool instead of boolean

Emil Velikov emil.l.velikov at gmail.com
Mon Jul 20 10:47:24 PDT 2015


On 18/07/15 16:02, samuel.pitoiset wrote:
> 
> 
> On 17/07/2015 23:08, Ilia Mirkin wrote:
>> On Fri, Jul 17, 2015 at 5:02 PM, Emil Velikov
>> <emil.l.velikov at gmail.com> wrote:
>>> On 16/07/15 22:39, Samuel Pitoiset wrote:
>>>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
>>>> ---
>>>>   src/gallium/drivers/nouveau/codegen/nv50_ir.h      |   2 +-
>>>>   .../drivers/nouveau/codegen/nv50_ir_driver.h       |  14 +--
>>>>   .../drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp  |   8 +-
>>>>   .../nouveau/codegen/nv50_ir_lowering_gm107.cpp     |   2 +-
>>>>   .../drivers/nouveau/codegen/nv50_ir_peephole.cpp   |   4 +-
>>>>   src/gallium/drivers/nouveau/nouveau_buffer.c       | 118
>>>> ++++++++++-----------
>>>>   src/gallium/drivers/nouveau/nouveau_buffer.h       |   6 +-
>>>>   src/gallium/drivers/nouveau/nouveau_context.h      |   4 +-
>>>>   src/gallium/drivers/nouveau/nouveau_fence.c        |  36 +++----
>>>>   src/gallium/drivers/nouveau/nouveau_fence.h        |  14 +--
>>>>   src/gallium/drivers/nouveau/nouveau_screen.c       |   6 +-
>>>>   src/gallium/drivers/nouveau/nouveau_screen.h       |   6 +-
>>>>   src/gallium/drivers/nouveau/nouveau_video.c        |  56 +++++-----
>>>>   src/gallium/drivers/nouveau/nouveau_winsys.h       |   4 +-
>>>>   src/gallium/drivers/nouveau/nv30/nv30_clear.c      |   2 +-
>>>>   src/gallium/drivers/nouveau/nv30/nv30_context.c    |   4 +-
>>>>   src/gallium/drivers/nouveau/nv30/nv30_context.h    |  10 +-
>>>>   src/gallium/drivers/nouveau/nv30/nv30_draw.c       |  22 ++--
>>>>   src/gallium/drivers/nouveau/nv30/nv30_fragprog.c   |   6 +-
>>>>   src/gallium/drivers/nouveau/nv30/nv30_miptree.c    |   4 +-
>>>>   src/gallium/drivers/nouveau/nv30/nv30_push.c       |   6 +-
>>>>   src/gallium/drivers/nouveau/nv30/nv30_query.c      |   4 +-
>>>>   src/gallium/drivers/nouveau/nv30/nv30_resource.c   |   4 +-
>>>>   src/gallium/drivers/nouveau/nv30/nv30_resource.h   |   2 +-
>>>>   src/gallium/drivers/nouveau/nv30/nv30_screen.c     |  10 +-
>>>>   src/gallium/drivers/nouveau/nv30/nv30_state.h      |   4 +-
>>>>   .../drivers/nouveau/nv30/nv30_state_validate.c     |   8 +-
>>>>   src/gallium/drivers/nouveau/nv30/nv30_transfer.c   |  54 +++++-----
>>>>   src/gallium/drivers/nouveau/nv30/nv30_vbo.c        |  26 ++---
>>>>   src/gallium/drivers/nouveau/nv30/nv30_vertprog.c   |  12 +--
>>>>   src/gallium/drivers/nouveau/nv30/nvfx_fragprog.c   |  46 ++++----
>>>>   src/gallium/drivers/nouveau/nv30/nvfx_shader.h     |   4 +-
>>>>   src/gallium/drivers/nouveau/nv30/nvfx_vertprog.c   |  50 ++++-----
>>>>   src/gallium/drivers/nouveau/nv50/nv50_blit.h       |  10 +-
>>>>   src/gallium/drivers/nouveau/nv50/nv50_context.c    |  14 +--
>>>>   src/gallium/drivers/nouveau/nv50/nv50_context.h    |  16 +--
>>>>   src/gallium/drivers/nouveau/nv50/nv50_miptree.c    |  26 ++---
>>>>   src/gallium/drivers/nouveau/nv50/nv50_program.c    |  18 ++--
>>>>   src/gallium/drivers/nouveau/nv50/nv50_program.h    |   6 +-
>>>>   src/gallium/drivers/nouveau/nv50/nv50_push.c       |   8 +-
>>>>   src/gallium/drivers/nouveau/nv50/nv50_query.c      |  38 +++----
>>>>   src/gallium/drivers/nouveau/nv50/nv50_resource.h   |   6 +-
>>>>   src/gallium/drivers/nouveau/nv50/nv50_screen.c     |  18 ++--
>>>>   src/gallium/drivers/nouveau/nv50/nv50_screen.h     |  16 +--
>>>>   .../drivers/nouveau/nv50/nv50_shader_state.c       |  18 ++--
>>>>   src/gallium/drivers/nouveau/nv50/nv50_state.c      |  24 ++---
>>>>   .../drivers/nouveau/nv50/nv50_state_validate.c     |  14 +--
>>>>   src/gallium/drivers/nouveau/nv50/nv50_stateobj.h   |   6 +-
>>>>   src/gallium/drivers/nouveau/nv50/nv50_surface.c    |  64 +++++------
>>>>   src/gallium/drivers/nouveau/nv50/nv50_tex.c        |  22 ++--
>>>>   src/gallium/drivers/nouveau/nv50/nv50_vbo.c        |  32 +++---
>>>>   src/gallium/drivers/nouveau/nvc0/nvc0_compute.c    |  24 ++---
>>>>   src/gallium/drivers/nouveau/nvc0/nvc0_compute.h    |   2 +-
>>>>   src/gallium/drivers/nouveau/nvc0/nvc0_context.c    |  12 +--
>>>>   src/gallium/drivers/nouveau/nvc0/nvc0_context.h    |  22 ++--
>>>>   src/gallium/drivers/nouveau/nvc0/nvc0_miptree.c    |  12 +--
>>>>   src/gallium/drivers/nouveau/nvc0/nvc0_program.c    |  18 ++--
>>>>   src/gallium/drivers/nouveau/nvc0/nvc0_program.h    |   6 +-
>>>>   src/gallium/drivers/nouveau/nvc0/nvc0_query.c      |  84
>>>> +++++++--------
>>>>   src/gallium/drivers/nouveau/nvc0/nvc0_screen.c     |  24 ++---
>>>>   src/gallium/drivers/nouveau/nvc0/nvc0_screen.h     |  16 +--
>>>>   .../drivers/nouveau/nvc0/nvc0_shader_state.c       |  12 +--
>>>>   src/gallium/drivers/nouveau/nvc0/nvc0_state.c      |  20 ++--
>>>>   .../drivers/nouveau/nvc0/nvc0_state_validate.c     |  20 ++--
>>>>   src/gallium/drivers/nouveau/nvc0/nvc0_stateobj.h   |   8 +-
>>>>   src/gallium/drivers/nouveau/nvc0/nvc0_surface.c    |  54 +++++-----
>>>>   src/gallium/drivers/nouveau/nvc0/nvc0_tex.c        |  38 +++----
>>>>   src/gallium/drivers/nouveau/nvc0/nvc0_transfer.c   |   8 +-
>>>>   src/gallium/drivers/nouveau/nvc0/nvc0_vbo.c        |  32 +++---
>>>>   .../drivers/nouveau/nvc0/nvc0_vbo_translate.c      |  18 ++--
>>>>   src/gallium/drivers/nouveau/nvc0/nve4_compute.c    |  24 ++---
>>>>   .../winsys/nouveau/drm/nouveau_drm_winsys.c        |   2 +-
>>>>   72 files changed, 685 insertions(+), 685 deletions(-)
>>>>
>>> Fwiw I'm like the idea, there is a small concern. There are some
>>> differences between boolean (char) and bool when it comes to implicit
>>> conversions and the likes (don't quote me on that). Iirc the intel guys
>>> did have some lovely experience chasing bugs due to that.
>>>
>>> Can you give this a quick piglit spin to confirm that it doesn't break
>>> things in a strange way.
>> In the past we've had bugs due to "dirty" booleans (i.e. not 1/0).
>> With bool that is much harder to construct (but possible!), and much
>> harder to debug in the cases where it does happen.
>>
>> However those cases are exclusively when you write to the memory
>> backing the bool in some sneaky way (aliasing, memcpy, etc). I'm
>> pretty sure we don't do any of this.
>>
>> FTR, I suggested making this change. But obviously should be run
>> through piglit and I'll have a look at it in detail later.
> 
> To make sure everything is okay, I'll run a full piglit test.
> 
Thanks guys. Just wanted to mention it rather than beating you about it.

-Emil



More information about the mesa-dev mailing list