Date: Mon, 6 Jan 2020 13:52:33 +0100 (CET)
From: Fabien COELHO <coelho@cri.ensmp.fr>
To: Robert Haas <robertmhaas@gmail.com>
cc: Vik Fearing <vik.fearing@2ndquadrant.com>, Tom Lane <tgl@sss.pgh.pa.us>,
    Stephen Frost <sfrost@snowman.net>,
    Dean Rasheed <dean.a.rasheed@gmail.com>,
    Chapman Flack <chap@anastigmatix.net>,
    PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Greatest Common Divisor
In-Reply-To: 
 <CA+TgmoaQXaOZtH-WcwMKbBOiiWtBvrw3giVN+XOvYieXG8QUXg@mail.gmail.com>
Message-ID: <alpine.DEB.2.21.2001061336390.24609@pseudo>
References: <adbd3e0b-e3f1-5bbc-21db-03caf1cef0f7@2ndquadrant.com>
 <alpine.DEB.2.21.1912281848540.24861@pseudo>
 <dbf86808-91fa-3262-f939-2ef3d38ec242@2ndquadrant.com>
 <alpine.DEB.2.21.1912290815520.889@pseudo>
 <5E08D947.9000902@anastigmatix.net>
 <alpine.DEB.2.21.1912291755510.14206@pseudo>
 <CAEZATCX9YgkWupVFcJ5ixkEJMyf2X5ZecN8K5r12oOB8mDHzWQ@mail.gmail.com>
 <20200102145712.GN3195@tamriel.snowman.net> <16242.1577977961@sss.pgh.pa.us>
 <6855727c-3836-21cc-18f9-e79227b99079@2ndquadrant.com>
 <alpine.DEB.2.21.2001032010470.12030@pseudo>
 <32b8af34-8f87-c30f-5555-2325bd140a59@2ndquadrant.com>
 <alpine.DEB.2.21.2001041003090.31333@pseudo>
 <13b5bbd5-6784-528f-9fd7-9fa43250be51@2ndquadrant.com>
 <alpine.DEB.2.21.2001042051170.6753@pseudo>
 <CA+TgmoaQXaOZtH-WcwMKbBOiiWtBvrw3giVN+XOvYieXG8QUXg@mail.gmail.com>
User-Agent: Alpine 2.21 (DEB 202 2017-01-01)
MIME-Version: 1.0
Content-Type: multipart/mixed;
 boundary="2056645051-1992234690-1578315153=:24609"
Precedence: bulk

  This message is in MIME format.  The first part should be readable text,
  while the remaining parts are likely unreadable without MIME-aware tools.

--2056645051-1992234690-1578315153=:24609
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8BIT


Hello Robert,

>>   if (arg1 == PG_INT32_MIN)
>>   if (arg2 == 0 || arg2 == PG_INT32_MIN)
>>
>> And possibly a "likely" on the while.
>
> I don't think decoration the code with likely() and unlikely() all
> over the place is a very good idea.

> Odds are good that we'll end up with a bunch that are actually 
> non-optimal, and nobody will ever figure it out because it's hard to 
> figure out.

My 0.02€: I'd tend to disagree.

Modern pipelined processors can take advantage of speculative execution on 
branches, so if you know which branch is the more likely it can help.

Obviously if you get it wrong it does not, but for the above cases it 
seems to me that they are rather straightforward.

It also provides some "this case is expected to be exceptional" semantics 
to people reading the code.

> I have a hard time believing that we're going to be much 
> worse off if we just write the code normally.

I think that your point applies to more general programming in postgres, 
but this is not the context here.

For low-level arithmetic code like this one, with tests and loops 
containing very few hardware instructions, I think that helping compiler 
optimizations is a good idea.

Maybe in the "while" the compiler would assume that it is going to loop 
anyway by default, so it may be less useful there.

-- 
Fabien.
--2056645051-1992234690-1578315153=:24609--