[Mesa-dev] [PATCH] i965: Replace default case with list of enum values.

[Matt Turner]

On Fri, Oct 30, 2015 at 5:19 PM, Ian Romanick <idr at freedesktop.org> wrote:
> On 10/29/2015 05:52 PM, Matt Turner wrote:
>> If we add a new file type, we'd like to get warnings if it's not
>> handled.
>>
>> Unfortuately, gcc seems to have bugs (see the XXX).
>
> Did you submit a GCC bug?

I did some research and found

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=28236
https://gcc.gnu.org/ml/gcc/2007-01/msg01183.html

If I understand correctly, an enum can be /any/ integral value in C
and that gcc would have to emit extra code to clamp the subject of the
switch to ensure that it was an enum value.

In C++, however, enums are fully different types and out-of-range
values for enums result in undefined behavior. Seems like the compiler
should be able to recognize this.

In practice, I found that moving the unreachable() into the switch and
returning an arbitrary value both shuts up the compiler *and*
generates smaller code. gcc *is* smart enough to recognize that the
code after the switch is unreachable in that case and avoid emitting
instructions for the useless return. How silly.

I've modified the patch locally to do that. The relevant hunks now look like:

diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
b/src/mesa/drivers/dri/i965/brw_fs.cpp
index 2d0acb9..5ab8c15 100644
--- a/src/mesa/drivers/dri/i965/brw_fs.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs.cpp

@@ -845,9 +843,8 @@ fs_inst::regs_read(int arg) const
                           REG_SIZE);
    case MRF:
       unreachable("MRF registers are not allowed as sources");
-   default:
-      unreachable("Invalid register file");
    }
+   return 0;
 }

 bool

diff --git a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
index 58bd23f..e207a77 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
@@ -42,9 +42,13 @@ static uint32_t brw_file_from_reg(fs_reg *reg)
       return BRW_MESSAGE_REGISTER_FILE;
    case IMM:
       return BRW_IMMEDIATE_VALUE;
-   default:
+   case BAD_FILE:
+   case HW_REG:
+   case ATTR:
+   case UNIFORM:
       unreachable("not reached");
    }
+   return 0;
 }