Message ID | 535F6BDE.3050801@redhat.com |
---|---|
State | New |
Headers | show |
> 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
diff --git a/sysdeps/x86_64/multiarch/init-arch.h b/sysdeps/x86_64/multiarch/init-arch.h index 813b6de..6449109 100644 --- a/sysdeps/x86_64/multiarch/init-arch.h +++ b/sysdeps/x86_64/multiarch/init-arch.h @@ -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 };