On Tue, Jan 16, 2018 at 08:50:24AM -0800, Andres Freund wrote: > On 2018-01-16 16:12:11 +0900, Michael Paquier wrote: > > On Fri, Feb 03, 2017 at 12:26:50AM +0000, Noah Misch wrote: > > > Since this emits double syncs with older xlc, I recommend instead > > > replacing > > > the whole thing with inline asm. As I opined in the last message of the > > > thread you linked above, the intrinsics provide little value as > > > abstractions > > > if one checks the generated code to deduce how to use them. Now that the > > > generated code is xlc-version-dependent, the port is better off with > > > compiler-independent asm like we have for ppc in s_lock.h. > > > > Could it be cleaner to just use __xlc_ver__ to avoid double syncs on > > past versions? I think that it would make the code more understandable > > than just listing directly the instructions. > > Given the quality of the intrinsics on AIX, see past commits and the > comment in the code quoted above, I think we're much better of doing > this via inline asm.
For me, verifiability is the crucial benefit of inline asm. Anyone with an architecture manual can thoroughly review an inline asm implementation. Given intrinsics and __xlc_ver__ conditionals, the same level of review requires access to every xlc version. > > As there have been other > > bug reports from Tony Reix who has been working on AIX with XLC 13.1 and > > that this thread got lost in the wild, I have added an entry in the next > > CF: > > https://commitfest.postgresql.org/17/1484/ The most recent patch version is Returned with Feedback. As a matter of procedure, I discourage creating commitfest entries as a tool to solicit new patch versions. If I were the author of a RwF patch, I would dislike finding a commitfest entry that I did not create with myself listed as author. If you do choose to proceed, the entry should be Waiting on Author. > > As Heikki is not around these days, Noah, could you provide a new > > version of the patch? This bug has been around for some time now, it > > would be nice to move on.. Not soon. Note that fixing this bug is just the start of accepting XLC 13.1 as a compiler of PostgreSQL. If we get a buildfarm member with a few dozen clean runs (blocked by, at a minimum, fixing this and the inlining bug), we'll have something. Until then, support for XLC 13.1 is an anti-feature. nm