[Mesa-dev] Mesa include guard style. (Was: [PATCH] i965/cfg: Remove redundant #pragma once.)

Francisco Jerez currojerez at riseup.net
Mon Mar 14 01:09:53 UTC 2016

Ian Romanick <idr at freedesktop.org> writes:

> On 03/11/2016 03:46 PM, Eric Anholt wrote:
>> Ian Romanick <idr at freedesktop.org> writes:
>>> On 03/10/2016 05:53 PM, Francisco Jerez wrote:
>>>> Iago Toral <itoral at igalia.com> writes:
>>>>> On Wed, 2016-03-09 at 19:04 -0800, Francisco Jerez wrote:
>>>>>> Matt Turner <mattst88 at gmail.com> writes:
>>>>>>> On Wed, Mar 9, 2016 at 1:37 PM, Francisco Jerez <currojerez at riseup.net> wrote:
>>>>>>>> Iago Toral <itoral at igalia.com> writes:
>>>>>>>>> On Tue, 2016-03-08 at 17:42 -0800, Francisco Jerez wrote:
>>>>>>>>>> brw_cfg.h already has include guards, remove the "#pragma once" which
>>>>>>>>>> is redundant and non-standard.
>>>>>>>>> FWIW, I think using both #pragma once and include guards is a way to
>>>>>>>>> keep portability while still getting the performance advantage of
>>>>>>>>> #pragma once where it is supported.
>>>>>>>> It's highly unlikely to make any significant difference on any
>>>>>>>> reasonably modern compiler.  I cannot measure any change in compilation
>>>>>>>> time locally from my cleanup.
>>>>>>>>> Also it seems that we do the same thing in many other files...
>>>>>>>> Really?  I'm not aware of any other file where we use both.
>>>>>>> There are quite a few in glsl/
>>>>>> Heh, apparently you're right.  Anyway it seems rather pointless to use
>>>>>> '#pragma once' in a bunch of scattered header files with the expectation
>>>>>> to gain some speed, the improvement from a single header file is so
>>>>>> minuscule (if it will make any difference at all on a modern compiler
>>>>>> and compilation workload, which I doubt) that we would have to use it
>>>>>> universally in order to have the chance to measure any improvement.
>>>>>> Can we please just decide for one of the include guard styles and use it
>>>>>> consistently?  Given that the majority of header files in the Mesa
>>>>>> codebase use old-school define guards, that it's the only standard
>>>>>> option, that it has well-defined semantics in presence of file copies
>>>>>> and hardlinks, and that the performance argument against it is rather
>>>>>> dubious (although I definitely find '#pragma once' prettier and more
>>>>>> concise), I'd vote for using preprocessor define guards universally.
>>>>>> What do other people think?
>>>>> I think we have to use define guards necessarily since #pragma once is
>>>>> not standard even it it has wide support. So the question is whether we
>>>>> want to use only define guards or define guards plus #pragma once. I am
>>>>> fine with doing only define guards as you propose.
>>>> *Shrug* I have the impression that the only real advantage of '#pragma
>>>> once' is that you no longer need to do the ifndef/define dance, so I
>>>> don't think I can see much benefit in doing both.
>>> Several compilers will cache the file name where '#pragma once' occurs
>>> and never read that file again.  A #include of a file previously seen
>>> with '#pragma once' becomes a no-op.  Since the file is never read, the
>>> compiler avoids all the I/O and the parsing.  That is true of MSVC and,
>>> I thought, some versions of GCC.  As Iago points out, some compilers
>>> ignore the #pragma altogether.  Since Mesa supports (or does it?) some
>>> of these compilers, we have to have the ifdef/define/endif guards.
>> Compilers have noticed that ifdef/define/endif is a thing and optimized
>> it, anyway.
>> https://gcc.gnu.org/onlinedocs/cppinternals/Guard-Macros.html
> That's cool!  I don't think GCC did that when I looked into this in
> 2010.  It sounds like the #pragma actually breaks the GCC optimization,
> so let's get rid of them all.

Shall I take that as an Acked-by for my patch?  It doesn't get rid of
them all but it's a start. ;)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160313/1f3ff86b/attachment.sig>

More information about the mesa-dev mailing list