| Message ID | 1348874411-28288-5-git-send-email-daniel.santos@pobox.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Fri, Sep 28, 2012 at 06:20:05PM -0500, Daniel Santos wrote: > --- a/include/linux/compiler-gcc4.h > +++ b/include/linux/compiler-gcc4.h > @@ -13,11 +13,11 @@ > #define __must_check __attribute__((warn_unused_result)) > #define __compiler_offsetof(a,b) __builtin_offsetof(a,b) > > -#if __GNUC_MINOR__ > 0 > +#if GCC_VERSION >= 40102 > # define __compiletime_object_size(obj) __builtin_object_size(obj, 0) > #endif You've changed the semantics of this one; if literally translated, this should have become #if GCC_VERSION >= 40100. If you intended to change that, could you please document why? And in any case, could you make that semantic change in a separate commit from the switch to GCC_VERSION? - Josh Triplett -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 09/28/2012 07:20 PM, Josh Triplett wrote: > On Fri, Sep 28, 2012 at 06:20:05PM -0500, Daniel Santos wrote: >> --- a/include/linux/compiler-gcc4.h >> +++ b/include/linux/compiler-gcc4.h >> @@ -13,11 +13,11 @@ >> #define __must_check __attribute__((warn_unused_result)) >> #define __compiler_offsetof(a,b) __builtin_offsetof(a,b) >> >> -#if __GNUC_MINOR__ > 0 >> +#if GCC_VERSION >= 40102 >> # define __compiletime_object_size(obj) __builtin_object_size(obj, 0) >> #endif > You've changed the semantics of this one; if literally translated, this > should have become #if GCC_VERSION >= 40100. If you intended to change > that, could you please document why? And in any case, could you make > that semantic change in a separate commit from the switch to > GCC_VERSION? hmm, it looks like somebody commented out the #error that would normally prevent that test from ever occurring on 4.1.0 or 4.1.1. When I had written this patch, it wasn't commented out and I had assumed that it was obvious from the context. > /* GCC 4.1.[01] miscompiles __weak */ > #ifdef __KERNEL__ > -# if __GNUC_MINOR__ == 1 && __GNUC_PATCHLEVEL__ <= 1 > +# if GCC_VERSION >= 40100 && GCC_VERSION <= 40101 > //# error Your version of gcc miscompiles the __weak directive > # endif > #endif > @@ -13,11 +13,11 @@ > #define __must_check __attribute__((warn_unused_result)) > #define __compiler_offsetof(a,b) __builtin_offsetof(a,b) > > -#if __GNUC_MINOR__ > 0 > +#if GCC_VERSION >= 40102 > # define __compiletime_object_size(obj) __builtin_object_size(obj, 0) > #endif I would say that commenting this out is bad if __weak is miscompiled. If we don't want to break the build, should we at least be defining __weak to something else? Daniel -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Sep 28, 2012 at 07:31:53PM -0500, Daniel Santos wrote: > On 09/28/2012 07:20 PM, Josh Triplett wrote: > > On Fri, Sep 28, 2012 at 06:20:05PM -0500, Daniel Santos wrote: > >> --- a/include/linux/compiler-gcc4.h > >> +++ b/include/linux/compiler-gcc4.h > >> @@ -13,11 +13,11 @@ > >> #define __must_check __attribute__((warn_unused_result)) > >> #define __compiler_offsetof(a,b) __builtin_offsetof(a,b) > >> > >> -#if __GNUC_MINOR__ > 0 > >> +#if GCC_VERSION >= 40102 > >> # define __compiletime_object_size(obj) __builtin_object_size(obj, 0) > >> #endif > > You've changed the semantics of this one; if literally translated, this > > should have become #if GCC_VERSION >= 40100. If you intended to change > > that, could you please document why? And in any case, could you make > > that semantic change in a separate commit from the switch to > > GCC_VERSION? > hmm, it looks like somebody commented out the #error that would normally > prevent that test from ever occurring on 4.1.0 or 4.1.1. > When I had written this patch, it wasn't commented out and I had assumed > that it was obvious from the context. GCC 4.1.0 and 4.1.1 miscompiling __weak has nothing to do with __compiletime_object_size; why should *this* version check exclude those versions? - Josh Triplett -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 28 Sep 2012, Josh Triplett wrote: > GCC 4.1.0 and 4.1.1 miscompiling __weak has nothing to do with > __compiletime_object_size; why should *this* version check exclude those > versions? > Agreed, we shouldn't be relying on any #error directives to fail the build and then try to factor those version numbers out of other directives; if the #error directive were ever to be removed (or commented out, like in this case), nobody would make the other directives inclusive of that now-allowed version where things like __compiletime_object_size() are valid. After __compiletime_object_size() is compiled for GCC_VERSION >= 40100, then: Acked-by: David Rientjes <rientjes@google.com> -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/linux/compiler-gcc3.h b/include/linux/compiler-gcc3.h index 37d4124..7d89feb 100644 --- a/include/linux/compiler-gcc3.h +++ b/include/linux/compiler-gcc3.h @@ -2,22 +2,22 @@ #error "Please don't include <linux/compiler-gcc3.h> directly, include <linux/compiler.h> instead." #endif -#if __GNUC_MINOR__ < 2 +#if GCC_VERSION < 30200 # error Sorry, your compiler is too old - please upgrade it. #endif -#if __GNUC_MINOR__ >= 3 +#if GCC_VERSION >= 30300 # define __used __attribute__((__used__)) #else # define __used __attribute__((__unused__)) #endif -#if __GNUC_MINOR__ >= 4 +#if GCC_VERSION >= 30400 #define __must_check __attribute__((warn_unused_result)) #endif #ifdef CONFIG_GCOV_KERNEL -# if __GNUC_MINOR__ < 4 +# if GCC_VERSION < 30400 # error "GCOV profiling support for gcc versions below 3.4 not included" # endif /* __GNUC_MINOR__ */ #endif /* CONFIG_GCOV_KERNEL */ diff --git a/include/linux/compiler-gcc4.h b/include/linux/compiler-gcc4.h index 4506d65..b44307d 100644 --- a/include/linux/compiler-gcc4.h +++ b/include/linux/compiler-gcc4.h @@ -4,7 +4,7 @@ /* GCC 4.1.[01] miscompiles __weak */ #ifdef __KERNEL__ -# if __GNUC_MINOR__ == 1 && __GNUC_PATCHLEVEL__ <= 1 +# if GCC_VERSION >= 40100 && GCC_VERSION <= 40101 //# error Your version of gcc miscompiles the __weak directive # endif #endif @@ -13,11 +13,11 @@ #define __must_check __attribute__((warn_unused_result)) #define __compiler_offsetof(a,b) __builtin_offsetof(a,b) -#if __GNUC_MINOR__ > 0 +#if GCC_VERSION >= 40102 # define __compiletime_object_size(obj) __builtin_object_size(obj, 0) #endif -#if __GNUC_MINOR__ >= 3 +#if GCC_VERSION >= 40300 /* Mark functions as cold. gcc will assume any path leading to a call to them will be unlikely. This means a lot of manual unlikely()s are unnecessary now for any paths leading to the usual suspects @@ -39,9 +39,9 @@ # define __compiletime_warning(message) __attribute__((warning(message))) # define __compiletime_error(message) __attribute__((error(message))) #endif /* __CHECKER__ */ -#endif /* __GNUC_MINOR__ >= 3 */ +#endif /* GCC_VERSION >= 40300 */ -#if __GNUC_MINOR__ >= 5 +#if GCC_VERSION >= 40500 /* * Mark a position in code as unreachable. This can be used to * suppress control flow warnings after asm blocks that transfer @@ -56,9 +56,9 @@ /* Mark a function definition as prohibited from being cloned. */ #define __noclone __attribute__((__noclone__)) -#endif /* __GNUC_MINOR__ >= 5 */ +#endif /* GCC_VERSION >= 40500 */ -#if __GNUC_MINOR__ >= 6 +#if GCC_VERSION >= 40600 /* * Tell the optimizer that something else uses this function or variable. */
Using GCC_VERSION reduces complexity, is easier to read and is GCC's recommended mechanism for doing version checks. (Just don't ask me why they didn't define it in the first place.) This also makes it easy to merge compiler-gcc{3,4}.h should somebody want to. Signed-off-by: Daniel Santos <daniel.santos@pobox.com> --- include/linux/compiler-gcc3.h | 8 ++++---- include/linux/compiler-gcc4.h | 14 +++++++------- 2 files changed, 11 insertions(+), 11 deletions(-)