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 <[email protected]> wrote: > > On Tue, 12 Dec 2023 at 08:17, Mike Beaton <[email protected]> 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/21656 Group Owner: [email protected] Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
