On Thu, May 25, 2023 at 12:42:05PM -0400, Alex Deucher wrote:
> On Thu, May 25, 2023 at 12:29 PM Nathan Chancellor <nathan@xxxxxxxxxx> wrote:
> >
> > On Thu, May 25, 2023 at 12:26:56PM -0400, Luben Tuikov wrote:
> > > On 2023-05-25 11:22, Nathan Chancellor wrote:
> > > > On Fri, May 19, 2023 at 06:14:38PM +0530, Srinivasan Shanmugam wrote:
> > > >> Silencing the compiler from below compilation error:
> > > >>
> > > >> drivers/gpu/drm/amd/amdgpu/mmhub_v1_8.c:704:23: error: variable 'mmhub_v1_8_mmea_err_status_reg' is not needed and will not be emitted [-Werror,-Wunneeded-internal-declaration]
> > > >> static const uint32_t mmhub_v1_8_mmea_err_status_reg[] = {
> > > >> ^
> > > >> 1 error generated.
> > > >>
> > > >> Mark the variable as __maybe_unused to make it clear to clang that this
> > > >> is expected, so there is no more warning.
> > > >>
> > > >> Cc: Christian König <christian.koenig@xxxxxxx>
> > > >> Cc: Lijo Lazar <lijo.lazar@xxxxxxx>
> > > >> Cc: Luben Tuikov <luben.tuikov@xxxxxxx>
> > > >> Cc: Alex Deucher <alexander.deucher@xxxxxxx>
> > > >> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@xxxxxxx>
> > > >
> > > > Traditionally, this attribute would go between the [] and =, but that is
> > > > a nit. Can someone please pick this up to unblock our builds on -next?
> > > >
> > > > Reviewed-by: Nathan Chancellor <nathan@xxxxxxxxxx>
> > >
> > > I'll pick this up, fix it, and submit to amd-staging-drm-next.
> >
> > Thanks a lot :)
> >
> > > Which -next are you referring to, Nathan?
> >
> > linux-next, this warning breaks the build when -Werror is enabled, such
> > as with allmodconfig:
> >
> > https://storage.tuxsuite.com/public/clangbuiltlinux/continuous-integration2/builds/2QHtlCTz2JL0yXNpRB5hVmiP9lq/build.log
> >
>
> Srinivasan has already pushed it. I'll push it out once CI has
> completed. We are trying to figure out the best way to enable -WERROR
> in our CI system as it is almost always broken depending on what
> compiler you are using. Also, I'm not sure fixing these is always
> better. A lot of these warnings seem spurious and in a lot of cases
> the "fix" doesn't really improve the code, it just silences a warning.
> As one of my coworkers put it, there is a reason warnings are not
> errors.
I do not necessarily disagree with that final sentiment but at the end
of the day, it is pointing out a potential problem ("this variable is
only used in a compile time context, is that what you intended or not?")
and the solution is either to fix the code so that it works as initially
intended or you silence the warning because you know it is not actually
a problem. There are always going to be false positives, otherwise they
would just always be hard errors, but that does not mean that they are
not worth listening to, which is why Linus insists on -Werror being a
thing. We can opt out of -Werror for our CI but that does not change the
fact it is default enabled with allmodconfig, so that is how most people
will test.
Cheers,
Nathan
