Re: Ryu floating point output patch

From: Andrew Gierth <andrew(at)tao11(dot)riddles(dot)org(dot)uk>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Ryu floating point output patch
Date: 2018-12-13 20:23:51
Message-ID: 87mup9192t.fsf@news-spur.riddles.org.uk
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

>>>>> "Andres" == Andres Freund <andres(at)anarazel(dot)de> writes:

>> From the upstream, I've taken only specific files which are
>> Boost-licensed, removed code not of interest to us, removed
>> C99-isms, applied project style for things like type names and use
>> of INT64CONST, removed some ad-hoc configuration #ifs in favour of
>> using values established by pg_config.h, and ran the whole thing
>> through pgindent and fixed up the resulting wreckage.

Andres> Removed C99isms? Why, we allow that these days? Did you mean
Andres> C11?

My understanding is that we do not allow // comments or mixed
declarations and code (other than loop control variables).

This code in particular was very heavily into using mixed declarations
and code throughout. Removing those was moderately painful.

Andres> What's with all the commented out code?

Just stuff from upstream I didn't take out.

Andres> I'm not sure that keeping close alignment with the original
Andres> codebase with things like that has much value given the extent
Andres> of the reformatting and functional changes?

Probably not, but otherwise I did not remove much in the way of upstream
comments or edit them except for formatting.

>> +/* These tables are generated by PrintDoubleLookupTable. */

Andres> This kind of reference is essentially dangling now, so perhaps
Andres> we should rewrite that?

Well, it's probably still useful to point out that they're generated
(though not by us); I guess it should make clear where the generator is.

>> +++ b/src/common/d2s_intrinsics.h

>> +static inline uint64
>> +umul128(const uint64 a, const uint64 b, uint64 *const productHi)
>> +{
>> + return _umul128(a, b, productHi);
>> +}

Andres> These are fairly generic names, perhaps we ought to prefix
Andres> them?

Maybe, but presumably nothing else is going to include this file.

--
Andrew (irc:RhodiumToad)

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2018-12-13 20:32:54 Re: Ryu floating point output patch
Previous Message Andres Freund 2018-12-13 20:21:58 Re: Reorganize collation lookup time and place