Peter Zijlstra
2022-May-19 11:16 UTC
[PATCH 1/3] sched/headers: Fix compilation error with GCC 12
On Mon, Apr 25, 2022 at 04:07:43PM +0200, Christophe de Dinechin wrote:> >> extern struct sched_class __begin_sched_classes[]; > >> extern struct sched_class __end_sched_classes[]; > >> > >> -#define sched_class_highest (__end_sched_classes - 1) > >> +/* > >> + * sched_class_highests is really __end_sched_classes - 1, but written in a way > >> + * that makes it clear that it is within __begin_sched_classes[] and not outside > >> + * of __end_sched_classes[]. > >> + */ > >> +#define sched_class_highest (__begin_sched_classes + \ > >> + (__end_sched_classes - __begin_sched_classes - 1)) > >> #define sched_class_lowest (__begin_sched_classes - 1) > >> > >> +/* The + 1 below places the pointers within the range of their array */ > >> #define for_class_range(class, _from, _to) \ > >> - for (class = (_from); class != (_to); class--) > >> + for (class = (_from); class + 1 != (_to) + 1; class--) > > > > Urgh, so now we get less readable code, > > You consider the original code readable?Yeah, because: x + y - x - 1 == y - 1, so wth would you want to write it with the x on. That's just silly.> It actually relies on a > precise layout that is not enforced in this code, not even documented, > but actually enforced by the linker script.It has a comment pointing at the linker script, and we have: /* Make sure the linker didn't screw up */ BUG_ON(&idle_sched_class + 1 != &fair_sched_class || &fair_sched_class + 1 != &rt_sched_class || &rt_sched_class + 1 != &dl_sched_class); #ifdef CONFIG_SMP BUG_ON(&dl_sched_class + 1 != &stop_sched_class); #endif On boot to verify the layout is as we expect.> > just because GCC is being > > stupid? > > I think that GCC is actually remarkably smart there. It tells you > that you are building pointers to A[] from B[], when there is a legit > way to say that the pointer is in A[] (which is what my patch does)We build with -fno-strict-aliasing, it must not assume anything like that, unless restrict is used. In this case, they're not two objects but the same one. Just because linker script can't really get us a sensible array definition.> > What's wrong with negative array indexes? memory is memory, stuff works. > > What?s wrong is that the compiler cannot prove theorems anymore. > These theorems are used to optimise code. When you write -1[B], the > compiler cannot optimise based on knowing this refers to A[B-A-1]. > > While at first, you might think that disabling a warning is a win, > what comes next is the compiler optimizing in a way you did not > anticipate, mysterious bugs showing up, and/or having to turn off some > potentially useful optimisation.We're usually fairly quick to call a compiler broken if doesn't do what we want it to. Dodgy optimizations go out the window real fast.