Fix -Wundef for FEATURE_INDEX_1.
Commit Message
While reviewing some code for Siddhesh and waiting for a build I had
time to do another -Wundef fix.
This is the simplest patch I can come up with to fix FEATURE_INDEX_1
-Wundef warnings. The constant definition is duplicated because we
can't use the enum constant in assembly nor in the other macros.
The bright side is that we end up with:
#ifdef __ASSEMBLER__
... Everything we need for assembly.
#endif
... Everything we need for non-assembly.
#endif
The down side is the slight duplication. I considered adding another
macro e.g.
#define __FEATURE_INDEX_1 0
...
# define FEATURE_INDEX_1 __FEATURE_INDEX_1
...
However, that just seemed ugly, so I left the duplication.
Any objections?
2014-04-29 Carlos O'Donell <carlos@redhat.com>
* sysdeps/x86_64/multiarch/init-arch.h [__ASSEMBLER__]:
Define FEATURE_INDEX_1 to 0.
[!__ASSEMBLER__]: Likewise.
---
Cheers,
Carlos.
Comments
> This is the simplest patch I can come up with to fix FEATURE_INDEX_1
> -Wundef warnings. The constant definition is duplicated because we
> can't use the enum constant in assembly nor in the other macros.
What does "nor in the other macros" mean? Of course a macro can be
based on arithmetic plus use of another constant that is usable in the
contexts where the macro is used. enum constants cannot be used in
assembly, nor in #if contexts. Did you mean to say that the other
macros are used in #if contexts and therefore must not use enum
constants?
> The down side is the slight duplication. I considered adding another
> macro e.g.
> #define __FEATURE_INDEX_1 0
> ...
> # define FEATURE_INDEX_1 __FEATURE_INDEX_1
> ...
> However, that just seemed ugly, so I left the duplication.
When "duplication" concretely means two instances of one macro whose
value is zero, then it seems less ugly than many other things. But in
the general case, duplication is worse than most other things.
In this situation, the question is, what are we getting from
FEATURE_INDEX_* being enum constants rather than macros at all?
This is internal-only code, so ease of maintenance is really the only
issue that we care much about. The general reason to have enum
constants for things is so that you can use them in the debugger rather
than either looking up values in the source by hand or relying on -g3
macro info. That is far less useful than the general case when its an
anonymous enum, so there is no type to which you can cast an integer
value to get the symbolic name trivially in gdb. Hence, literally the
only benefit is typing FEATURE_INDEX_1 in gdb. If we cared about that,
we'd make all the macros in init-arch.h be enum constants. So I think
we just aren't getting anything worthwhile from having it be an enum.
Let's eliminate the duplication by just having one unconditional #define.
Thanks,
Roland
@@ -49,6 +49,11 @@
#ifdef __ASSEMBLER__
+/* The feature index constants must match those used in the non-assembly
+ below. We can't use a unified definition because the assembly won't
+ accept enum constants, nor will other macros. */
+# define FEATURE_INDEX_1 0
+
# include <ifunc-defines.h>
# define index_SSE2 COMMON_CPUID_INDEX_1*CPUID_SIZE+CPUID_EDX_OFFSET
@@ -85,6 +90,7 @@ enum
enum
{
FEATURE_INDEX_1 = 0,
+# define FEATURE_INDEX_1 0
/* Keep the following line at the end. */
FEATURE_INDEX_MAX
};