Re: [edk2-devel] [PATCH] BaseTools/tools_def: Disable unneeded-internal-declaration warning in CLANGPDB

["Mike Beaton" <mjsbeaton@gmail.com>]

I have realised that this was already fixed (i.e. allowing keeping the
warning) in Acidanthera fork of EDK-II. Discussed here
https://bugzilla.tianocore.org/show_bug.cgi?id=3704 - includes the fix
in question and other fixes for newer gcc as well. I'll post a new
patch to the list proposing just the relevant fix for clang.

On Tue, 12 Dec 2023 at 07:49, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Tue, 12 Dec 2023 at 08:17, Mike Beaton <mjsbeaton@gmail.com> wrote:
> >
> > > > A completely different approach, which allows clang to spot that the
> > > > usage has been 'optimised away' and so to not complain (and therefore
> > > > allows us to re-enable the warning in CLANGDWARF as well), is the
> > > > following:
> > > >
> > > > --- a/MdePkg/Include/Library/DebugLib.h
> > > > +++ b/MdePkg/Include/Library/DebugLib.h
> > > > @@ -426,7 +426,10 @@ UnitTestDebugAssert (
> > > >        }                            \
> > > >      } while (FALSE)
> > > >  #else
> > > > -#define DEBUG(Expression)
> > > > +#define DEBUG(Expression)        \
> > > > +    if (FALSE) {                   \
> > > > +      _DEBUG (Expression);         \
> > > > +    }
> > > >  #endif
> > > >
> > > >  /**
> > > >
> > >
> > > But will this not litter the object files with a bunch of format strings
> > > etc?
> >
> > Yes. Which would be optimized away at link time. (Or rather, I believe
> > so, would need further tests to confirm exactly what is optimized away
> > when.)
> >
>
> Link time optimization does not usually optimize away assets at this
> granularity. Even if --gc-sections is set, the only thing it can
> optimize away is an entire input section.
>
> However, the compiler should be smart enough not to emit any
> references to format strings etc in the first place, as it knows the
> condition can never become true (but NOOPT builds should retain them).
>
> > > It feels like, for CLANGPDB (and maybe CLANGDWARF), the RELEASE target
> > > should not define MDEPKG_NDEBUG, but make sure (instead) that
> > > DebugPrintEnabled() evals to FALSE. If PcdDebugPropertyMask is
> > > fixed-at-build, then DebugPrintEnabled() should be possible to evaluate
> > > at compile time; is that right? (At least for clang?)
> >
> > Yes, that is my understanding of how compile-time Pcds work too - but
> > wouldn't the net result be the same as what I suggested?
>
> It depends on the kind of access. For PCDs that are fixed-at-build
> only, the FixedPcdGet###() accessors will evaluate to constant
> preprocessor expressions, allowing the compiler to do optimizations.
> The ordinary PcdGet###() routines will not produce compile time
> constant expressions in the same way, so there, all the logic is
> retained (again, modulo LTO)


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112362): https://edk2.groups.io/g/devel/message/112362
Mute This Topic: https://groups.io/mt/103087794/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-