Noah Misch <noah@leadboat.com> writes:
> On Wed, Oct 09, 2019 at 01:15:29PM -0400, Tom Lane wrote:
>> * I still think that the added configure test is a waste of build cycles.
>> It'd be sufficient to test "#ifdef HAVE__BUILTIN_CONSTANT_P" where you
>> are testing HAVE_I_CONSTRAINT__BUILTIN_CONSTANT_P, because our previous
>> buildfarm go-round with this showed that all supported compilers
>> interpret "i" this way.
> xlc does not interpret "i" that way:
> https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=hornet&dt=2019-09-14%2016%3A42%3A32&stg=config
Hm, how'd I miss that while looking at the buildfarm results? Anyway,
you're clearly right on this one --- objection withdrawn.
>> * I really dislike building the asm calls with macros as you've done
>> here.
> For a macro local to one C file, I think readability is the relevant metric.
> In particular, it would be wrong to add arguments to make these more like
> header file macros. I think the macros make the code somewhat more readable,
> and you think they make the code less readable. I have removed the macros.
Personally I definitely find this way more readable, though I agree
beauty is in the eye of the beholder.
I've checked that this patch set compiles and passes regression tests
on my old Apple machines, so it's good to go as far as I can see.
regards, tom lane