[Swan-dev] type_t, type() and TYPE() in initializers

Andrew Cagney andrew.cagney at gmail.com
Sat Jan 19 17:57:31 UTC 2019


On Sat, 19 Jan 2019 at 03:51, D. Hugh Redelmeier <hugh at mimosa.com> wrote:
>
> | From: Andrew Cagney <andrew.cagney at gmail.com>
>
> | I'm sure this has come up before -  the discussion concluded that a
> | cast within a static initializer probably isn't technically static.
>
> A compound literal starts with what looks like a cast but it isn't a
> cast.
>
> On the other hand, a compound literal actually creates an object, not
> a value.  I hadn't realized this.  Not to my taste but potentially
> useful.

> It doesn't make sense to initialize a static with an assignment from
> an object.  Amusingly, it does make sense to initialize a static with
> the address of a compound literal!
>
> I think GCC makes sense of initializing a static with a compound literal
> unless it has told to be strict in some way.  We always want GCC
> strictness so more of our errors will be caught.

Surely it is better to write code compliant with the language spec,
and not according to what a specific compiler seems to accept (unless
there's a really compelling case and evidence that LLVM also accepts
the same construct)?

Here, compound literals aren't valid in static initializers but gcc
_sometimes_ accepts them.  We need a simple consistent way of
achieving the same result.


> | Through trial and error (read, being repeatedly pinged for the same
> | mistake on IRC cf eb3ba97984910933b3ce9ff3155c2932b48597a0), I've
> | found the following works best:
> |
> |  - lower case function or variable - here chunk(), empty_chunk, ... -
> | use this in all dynamic code
> | -  upper case macro - here CHUNK(), EMPTY_CHUNK, ... - use this in all
> | static code
>
> I take it by "use this in all static code" you really mean "use this
> as an initializer".  In other words, it isn't an expression.
>
> That's not our past convention and I don't immediately see why to use
> it.

Er..  But our past convention was to:
- ban dynamic initializers
- require static initializers to be fully populated and not use named fields
So prior to me using these new language features, and better
abstracting primitives like realtime_t, this hadn't even come up.  And
in doing this, I started following a convention you clearly don't
like.

> REALTIME_EPOCH is not a constant: it cannot be used in a normal
> expression.  It is an initializer.  And it is used only once: hardly
> worth defining.  But writing it in upper case does conform: it doesn't
> work like a function.

Off the top of my head there's REALTIME_EPOCH, MONOTIME_EPOCH, and EMPTY_CHUNK.

So take a step back and look at them as a group.

> REALTIME_EPOCH is used as the initializer for the definition of the
> constant object "realtime_epoch".  realtime_epoch is used once.
>
> It would be quite reasonable to define REALTIME_EPOCH as a compound
> literal.  And then use it instead of realtime_epoch.  It would still
> only be used once.

However, I can be confident that:

const chunk_t empty_chunk = EMPTY_CHUNK;

yields the same value as EMPTY_CHUNK used elsewhere.

And as I've had to point out in the past, just because something isn't
used, or is only used once is not good reason to delete it.

> | When I tried to cast something like EMPTY_CHUNK so that it, instead of
> | empty_chunk, could be used (a foolish premature optimisation on my
> | part), I was invariably forced to either:
> |
> | - define a further macro with no cast - above avoids needing to do this
> | - or hard-wire those cases - as has happened here - defeating the
> | point of the abstraction
>
> I don't understand this point.
>
> EMPTY_CHUNK is a compound literal.  It works everywhere empty_chunk
> would work.  Neither would work as a static initializer.
>
> There is one exception.  The address of empty_chunk would always be
> the same whereas the address of EMPTY_CHUNK would always be different.
> But I don't think any of our code would care.
>
> | I don't think performance and/or static analysers are an issue:
> |
> |  - given their price, I'd be really surprised if a static analyser,
> | which looks at the entire code base, couldn't figure out that:
> |   return empty_chunk;
> | and
> |   return (chunk_t) { NULL,0, }
> | are the same
>
> As far as I know we don't run the compiler in "whole program" mode.

GCC isn't a static analyser.

You offer no evidence that GCC can make use of this.  As you point out
it isn't doing whole program static analysis so can't see 'NULL' being
returned from a function in another file.

You offer no evidence to support this making pluto materially faster.
OTOH, I can point to specific performance problems and they all have
zero to do with these sorts of changes.


> The static analyzer that we do use can do a better job with the
> EMPTY_CHUNK version than the empty_chunk version.
>
> Better means: potentially better code and potentially better
> diagnostics.
>
> I would expect slightly better object code if every rvalue-only use of
> empty_chunk were replaced by EMPTY_CHUNK.
>
> | - pluto spends huge amounts of time executing assembler computing DH
> | and RSA,, the above is in the wash
>
> For time, yes.  But given the size of the NSS library, it is probably
> true for space too (NSS might be shared with other processes).
>
> | OTOH,  I can print empty_chunk (but not EMPTY_CHUNK) and can reliably
> | breakpoint and step over a chunk() function (but not the alternatives,
> | at least not reliably)
>
> Why would you want to?  These are tiny little transparent operators.
> You know from inspection just what they do.

Because it makes debugging easier.  I find it troubling that I need to
answer this.

Andrew


More information about the Swan-dev mailing list