<div dir="ltr">This is a nice improvement over the explicit cast, which is how I've always done it in the past - which is the ugly part of an otherwise great method for flags. Also I use & a lot with enum for clearing bits.</div> <div class="gmail_extra"> <div class="gmail_quote">On Fri, Aug 23, 2013 at 3:18 PM, Paul Berry <<a href="mailto:stereotype441@gmail.com" target="_blank">stereotype441@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">(From a suggestion by Francisco Jerez) If an enum represents a bitfield of flags, e.g.: enum E { A = 1, B = 2, C = 4, D = 8, }; then C++ normally prohibits statements like this: enum E x = A | B; because A and B are implicitly converted to ints before OR-ing them, and an int can't be stored in an enum without a type cast. C, on the other hand, allows an int to be implicitly converted to an enum without casting. In the past we've dealt with this situation by storing flag bitfields as ints. This avoids ugly casting at the expense of some type safety that C++ would normally have offered (e.g. we get no warning if we accidentally use the wrong enum type). However, we can get the best of both worlds if we override the | operator. The ugly casting is confined to the operator overload, and we still get the benefit of C++ making sure we don't use the wrong enum type. The disadvantages are that (a) we need an explicit enum value for 0, and (b) we can't use related operators like |= unless we define additional overloads. So what do folks think? Is it worth it? Cc: Francisco Jerez <<a href="mailto:currojerez@riseup.net">currojerez@riseup.net</a>> --- src/mesa/drivers/dri/i965/brw_clip.h | 2 +- src/mesa/drivers/dri/i965/brw_clip_util.c | 2 +- src/mesa/drivers/dri/i965/brw_eu.h | 16 +++++++++++++++- src/mesa/drivers/dri/i965/brw_eu_emit.c | 4 ++-- src/mesa/drivers/dri/i965/brw_sf_emit.c | 12 ++++++++---- src/mesa/drivers/dri/i965/brw_vec4.h | 2 +- src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp | 3 ++- 7 files changed, 30 insertions(+), 11 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_clip.h b/src/mesa/drivers/dri/i965/brw_clip.h index 5af0ad3..41f5c75 100644 --- a/src/mesa/drivers/dri/i965/brw_clip.h +++ b/src/mesa/drivers/dri/i965/brw_clip.h @@ -173,7 +173,7 @@ void brw_clip_init_planes( struct brw_clip_compile *c ); void brw_clip_emit_vue(struct brw_clip_compile *c, struct brw_indirect vert, - unsigned flags, + enum brw_urb_write_flags flags, GLuint header); void brw_clip_kill_thread(struct brw_clip_compile *c); diff --git a/src/mesa/drivers/dri/i965/brw_clip_util.c b/src/mesa/drivers/dri/i965/brw_clip_util.c index d5c50d7..24d053e 100644 --- a/src/mesa/drivers/dri/i965/brw_clip_util.c +++ b/src/mesa/drivers/dri/i965/brw_clip_util.c @@ -313,7 +313,7 @@ void brw_clip_interp_vertex( struct brw_clip_compile *c, void brw_clip_emit_vue(struct brw_clip_compile *c, struct brw_indirect vert, - unsigned flags, + enum brw_urb_write_flags flags, GLuint header) { struct brw_compile *p = &c->func; diff --git a/src/mesa/drivers/dri/i965/brw_eu.h b/src/mesa/drivers/dri/i965/brw_eu.h index 9053ea2..069b223 100644 --- a/src/mesa/drivers/dri/i965/brw_eu.h +++ b/src/mesa/drivers/dri/i965/brw_eu.h @@ -229,6 +229,8 @@ void brw_set_dp_write_message(struct brw_compile *p, GLuint send_commit_msg); enum brw_urb_write_flags { + BRW_URB_WRITE_NO_FLAGS = 0, + /** * Causes a new URB entry to be allocated, and its address stored in the * destination register (gen < 7). @@ -271,11 +273,23 @@ enum brw_urb_write_flags { BRW_URB_WRITE_ALLOCATE | BRW_URB_WRITE_COMPLETE, }; +#ifdef __cplusplus +/** + * Allow brw_urb_write_flags enums to be ORed together (normally C++ wouldn't + * allow this without a type cast). + */ +inline enum brw_urb_write_flags +operator|(enum brw_urb_write_flags x, enum brw_urb_write_flags y) +{ + return (enum brw_urb_write_flags) ((int) x | (int) y); +} +#endif + void brw_urb_WRITE(struct brw_compile *p, struct brw_reg dest, GLuint msg_reg_nr, struct brw_reg src0, - unsigned flags, + enum brw_urb_write_flags flags, GLuint msg_length, GLuint response_length, GLuint offset, diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c b/src/mesa/drivers/dri/i965/brw_eu_emit.c index b55b57e..ecf8597 100644 --- a/src/mesa/drivers/dri/i965/brw_eu_emit.c +++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c @@ -515,7 +515,7 @@ static void brw_set_ff_sync_message(struct brw_compile *p, static void brw_set_urb_message( struct brw_compile *p, struct brw_instruction *insn, - unsigned flags, + enum brw_urb_write_flags flags, GLuint msg_length, GLuint response_length, GLuint offset, @@ -2213,7 +2213,7 @@ void brw_urb_WRITE(struct brw_compile *p, struct brw_reg dest, GLuint msg_reg_nr, struct brw_reg src0, - unsigned flags, + enum brw_urb_write_flags flags, GLuint msg_length, GLuint response_length, GLuint offset, diff --git a/src/mesa/drivers/dri/i965/brw_sf_emit.c b/src/mesa/drivers/dri/i965/brw_sf_emit.c index d329bef..b206797 100644 --- a/src/mesa/drivers/dri/i965/brw_sf_emit.c +++ b/src/mesa/drivers/dri/i965/brw_sf_emit.c @@ -491,7 +491,8 @@ void brw_emit_tri_setup(struct brw_sf_compile *c, bool allocate) brw_null_reg(), 0, brw_vec8_grf(0, 0), /* r0, will be copied to m0 */ - last ? BRW_URB_WRITE_EOT_COMPLETE : 0, + last ? BRW_URB_WRITE_EOT_COMPLETE + : BRW_URB_WRITE_NO_FLAGS, 4, /* msg len */ 0, /* response len */ i*4, /* offset */ @@ -562,7 +563,8 @@ void brw_emit_line_setup(struct brw_sf_compile *c, bool allocate) brw_null_reg(), 0, brw_vec8_grf(0, 0), - last ? BRW_URB_WRITE_EOT_COMPLETE : 0, + last ? BRW_URB_WRITE_EOT_COMPLETE + : BRW_URB_WRITE_NO_FLAGS, 4, /* msg len */ 0, /* response len */ i*4, /* urb destination offset */ @@ -649,7 +651,8 @@ void brw_emit_point_sprite_setup(struct brw_sf_compile *c, bool allocate) brw_null_reg(), 0, brw_vec8_grf(0, 0), - last ? BRW_URB_WRITE_EOT_COMPLETE : 0, + last ? BRW_URB_WRITE_EOT_COMPLETE + : BRW_URB_WRITE_NO_FLAGS, 4, /* msg len */ 0, /* response len */ i*4, /* urb destination offset */ @@ -706,7 +709,8 @@ void brw_emit_point_setup(struct brw_sf_compile *c, bool allocate) brw_null_reg(), 0, brw_vec8_grf(0, 0), - last ? BRW_URB_WRITE_EOT_COMPLETE : 0, + last ? BRW_URB_WRITE_EOT_COMPLETE + : BRW_URB_WRITE_NO_FLAGS, 4, /* msg len */ 0, /* response len */ i*4, /* urb destination offset */ diff --git a/src/mesa/drivers/dri/i965/brw_vec4.h b/src/mesa/drivers/dri/i965/brw_vec4.h index 5d8f0bf..0cb0347 100644 --- a/src/mesa/drivers/dri/i965/brw_vec4.h +++ b/src/mesa/drivers/dri/i965/brw_vec4.h @@ -222,7 +222,7 @@ public: int target; /**< MRT target. */ bool shadow_compare; - unsigned urb_write_flags; + enum brw_urb_write_flags urb_write_flags; bool header_present; int mlen; /**< SEND message length */ int base_mrf; /**< First MRF in the SEND message, if mlen is nonzero. */ diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp index 72841e7..d0959b7 100644 --- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp +++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp @@ -2788,7 +2788,8 @@ vec4_vs_visitor::emit_urb_write_opcode(bool complete) } vec4_instruction *inst = emit(VS_OPCODE_URB_WRITE); - inst->urb_write_flags = complete ? BRW_URB_WRITE_EOT_COMPLETE : 0; + inst->urb_write_flags = complete ? + BRW_URB_WRITE_EOT_COMPLETE : BRW_URB_WRITE_NO_FLAGS; return inst; } -- 1.8.3.4 _______________________________________________ mesa-dev mailing list <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> <a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev" target="_blank">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div> </div>