On 07/09/15 19:35, Bill Paul wrote: > Of all the gin joints in all the towns in all the world, Laszlo Ersek had to > walk into mine at 22:35:37 on Wednesday 08 July 2015 and say: > > [...] > >>> Last year I spent some time to get gcc -flto working properly with EDK2. >>> At that time, the people here were busy with other things and there >>> didn't seem to be a lot of interest in gcc link time optimization. So I >>> never submitted a patch. Maybe it is time to revive this effort. Here is >>> what I >>> >>> found: >>> http://notabs.org/uefi/gcc-lto.htm >> >> I apologize if I should remember this from last year, but I don't. Sorry >> about that. Do you think you could revive it? The space savings would be >> nice, but more importantly (to me), that would allow us to "opt out" of >> MDEPKG_NDEBUG in at least OvmfPkg. (Based on Andrew's description, I now >> believe that opting out of MDEPKG_NDEBUG is a good thing -- it would >> allow the compiler to check more things, rather than "hiding" those >> things with the preprocessor.) >> >> In the article you wrote, >> >>> gcc link time optimization exposes a few warnings that do not occur >>> with the standard builds >> >> and I guess Bill hinted that he saw similar warnings, and at least one >> of those indicated a real problem?... So maybe we should find and fix >> these for good. > > I went back to check on this again -- I changed the OVMF build rules to leave > MDEPKG_NDEBUG turned off to avoid any of the warnings related to ASSERT() et. > al. -- and found exactly one case: > > /home/wpaul/edk/edk2/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c: In function > 'QemuFwCfgFindFile': > /home/wpaul/edk/edk2/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c:280:12: > error: variable 'FileReserved' set but not used [-Werror=unused-but-set- > variable] > UINT16 FileReserved; > > The code in question is: > > [...] > for (Idx = 0; Idx < Count; ++Idx) { > UINT32 FileSize; > UINT16 FileSelect; > UINT16 FileReserved; > CHAR8 FName[QEMU_FW_CFG_FNAME_SIZE]; > > FileSize = QemuFwCfgRead32 (); > FileSelect = QemuFwCfgRead16 (); > FileReserved = QemuFwCfgRead16 (); > InternalQemuFwCfgReadBytes (sizeof (FName), FName); > > if (AsciiStrCmp (Name, FName) == 0) { > *Item = SwapBytes16 (FileSelect); > *Size = SwapBytes32 (FileSize); > return RETURN_SUCCESS; > } > } > [...] > > I guess this isn't really a bug. When iterating over these file structures, > it's necessary to read in this reserved field in order to consume the header > fields and advance to the start of the file name field, but since the > reserved > field is not actually used the compiler complains. > > I got around the warning by doing this: > > [...] > FileSize = QemuFwCfgRead32 (); > FileSelect = QemuFwCfgRead16 (); > FileReserved = QemuFwCfgRead16 (); > (void)FileReserved; /* Force a do-nothing reference. */ > InternalQemuFwCfgReadBytes (sizeof (FName), FName); > [...]
Right. One could even say, (VOID)QemuFwCfgRead16 (); but the way you fixed it is much easier to read. (Just VOID should be spelled upper case, and, theoretically, there should be a space after "(VOID)", but that latter rule I keep actively subverting, because it is wrong.) How do you feel about submitting a patch for this? :) Thanks Laszlo > > -Bill > ------------------------------------------------------------------------------ Don't Limit Your Business. Reach for the Cloud. GigeNET's Cloud Solutions provide you with the tools and support that you need to offload your IT needs and focus on growing your business. Configured For All Businesses. Start Your Cloud Today. https://www.gigenetcloud.com/ _______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel
