Re: [PATCH 1/7] Compiler Attributes: remove unused attributes

[Nick Desaulniers]

On Fri, Aug 31, 2018 at 1:23 PM Miguel Ojeda
<miguel.ojeda.sandonis@xxxxxxxxx> wrote:
>
> On Fri, Aug 31, 2018 at 8:43 PM, Nick Desaulniers
> <ndesaulniers@xxxxxxxxxx> wrote:
> > On Fri, Aug 31, 2018 at 10:28 AM Joe Perches <joe@xxxxxxxxxxx> wrote:
> >>
> >> On Fri, 2018-08-31 at 19:05 +0200, Miguel Ojeda wrote:
> >> > __optimize and __deprecate_for_modules are unused in
> >> > the whole kernel tree. Simply drop them.
> >>
> >> Nice series, thanks Miguel.
> >>
> >> It'd be good to have a cover letter for the series.
> >>
> >> And I believe there should be the equivalent of:
> >>
> >> #if GCC_VERSION < 40600
> >> # error Sorry, your compiler is too old - please upgrade it.
> >> #endif
> >>
> >> for compiler-intel.h and compiler-clang.h so that
> >> each supported compiler minimum version is checked.
> >>
> >> Is it clang > 13 and icc > 3 ?
> >
> > Eh, I'm not sure I want to commit yet to a specific minimal version of
> > Clang.  Right now, we're fixing things so depending on arch's and
> > configs, the answer might be Top of Tree clang builds.  For Pixel, we
> > shipped with Clang-4, but pretty quickly we needed Clang-5.
> > https://lkml.org/lkml/2017/11/22/943
> >
> > I had sent patches previously for detecting clang version from the C
> > preprocessor, maybe I should dust those off, then commit to clang 5.
>
> In my opinion, even if you require clang 7, that is fine, as long as
> we get a working build mainlined.

Clang 7 is aggressive.  I need to think more about how to call out
when a specific set of configs for a given arch requires a compiler
upgrade, without ending up with combinatoral explosion.  I don't want
to cross that bridge with this patch set.

>
> By the way, I am testing the series with clang 8 (2018-08-14) (after
> reverting e501ce957a78), and it seems to work. Hopefully that makes
> you happy! ;-)

That makes me very happy.  It indeed does produce a run-able
executable, for some subset of configs, but can't be relied upon until
we complete our implementation (WIP).  CC me on any bugs you find for
your configs.  I'm also trying to keep a handle on things in
https://github.com/ClangBuiltLinux/linux/issues.

>
> >
> > I don't think minimal supported versions are required for these clean
> > ups, and would not block these patches from landing on that.
> >
> > Also, haven't found anyone using ICC yet to comment on minimal version
> > requirements.
>
> For clang, by the way, __naked should go out of -gcc.h.

Yep, Arnd's note in the other thread was a valuable insight and I agree with it.

> I guess that
> is breaking ARM clang builds at the moment (didn't check)?

Huh?


> I will
> include the move for v3.
>
> Cheers,
> Miguel



-- 
Thanks,
~Nick Desaulniers