public inbox for [email protected]
help / color / mirror / Atom feedFrom: Heikki Linnakangas <[email protected]>
To: Srirama Kucherlapati <[email protected]>
To: wenhui qiu <[email protected]>
To: [email protected] <[email protected]>
To: [email protected] <[email protected]>
Cc: Robert Haas <[email protected]>
Cc: Bruce Momjian <[email protected]>
Cc: Peter Eisentraut <[email protected]>
Cc: Alvaro Herrera <[email protected]>
Cc: Laurenz Albe <[email protected]>
Cc: Noah Misch <[email protected]>
Cc: Michael Paquier <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Thomas Munro <[email protected]>
Cc: [email protected] <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: Tristan Partin <[email protected]>
Subject: Re: AIX support
Date: Fri, 4 Apr 2025 23:44:00 +0300
Message-ID: <[email protected]> (raw)
In-Reply-To: <CY8PR15MB56020EB4076E7AACBE4A78A7DBA92@CY8PR15MB5602.namprd15.prod.outlook.com>
References: <CY5PR11MB639218D04CFAED0FC09ED9DFFDED2@CY5PR11MB6392.namprd11.prod.outlook.com>
<CY8PR15MB5602F897E478A24903B480C4DB292@CY8PR15MB5602.namprd15.prod.outlook.com>
<CY8PR15MB56025F74BDA3E078288A6A0ADB372@CY8PR15MB5602.namprd15.prod.outlook.com>
<CY8PR15MB56026092DF51E9D300557B5ADB022@CY8PR15MB5602.namprd15.prod.outlook.com>
<[email protected]>
<CY8PR15MB56027303B385970CEBF22107DBF12@CY8PR15MB5602.namprd15.prod.outlook.com>
<CY8PR15MB5602D669251C0A50BA018C96DBD52@CY8PR15MB5602.namprd15.prod.outlook.com>
<CA+TgmoZf2nGA5QJFreLFmeWngvZdbCcZR7aw00vxnwr0mCiK4A@mail.gmail.com>
<DS0PR15MB56239360EB759C70AC514845DBDF2@DS0PR15MB5623.namprd15.prod.outlook.com>
<CAGjGUAJxeYgZR1JLs3o3Lc6wsmDiHK5s3OAq8obJnk+9c9nE0g@mail.gmail.com>
<CY8PR15MB560263740FABD624904C4923DBAE2@CY8PR15MB5602.namprd15.prod.outlook.com>
<[email protected]>
<CY8PR15MB56020EB4076E7AACBE4A78A7DBA92@CY8PR15MB5602.namprd15.prod.outlook.com>
On 04/04/2025 22:31, Srirama Kucherlapati wrote:
> - src/backend/port/aix/mkldexport.sh>
> - When building shared libraries from various archives on AIX, we encounter a
> situation where symbols are not exported. To resolve this, we require an export
> file. For instance, the command is used to export symbols.
>
> gcc -shared libtest.so libtest.a -Wl,-bE:test.exp
>
> However, if we directly provide object files in the command line instead of an
> archive, the symbols will be exported automatically, as demonstrated by the command
>
> gcc -shared libtest.so test1.o test2.o test3.o.
Oh, that's interesting. So if we do that, we don't need mkldepxort.sh
anymore?
> - WRT to the MEMSET_LOOP_LIMIT flag, this is set to “0”, which would
> internally use
>
> The system call memset() as mentioned in the below link as well
>
> https://www.postgresql.org/message-
> id/20060203135315.E08B09DC816%40postgresql.org <https://
> www.postgresql.org/message-id/20060203135315.E08B09DC816%40postgresql.org>
Yes, I understand what it does. But why? Whatever benchmarking was done
back in 2006 by is no longer relevant.
> With all the above changes we have built and ran the tests. As of now we see
>
> there is only one test case that is failing, which seems to have been
>
> introduced recently. And this might not be related to the above changes as
>
> earlier there were no test cases failing.
>
> 64 not ok 12 + float8 235 ms
>
> 297 # 1 of 226 tests failed.
>
> 20 +ERROR: value out of range: overflow
>
> 21 -- test overflow/underflow handling
>
> 22 SELECT gamma(float8 '-infinity');
>
> 23 ERROR: value out of range: overflow
Yeah, that function is new. I don't know if it's a pre-existing problem
or something specific to AIX. Please check the archives for prior
discussion on that; if you can reproduce it, maybe you can help to fix it?
> 12 files changed, 224 insertions(+), 20 deletions(-)
I'm glad to see this patch shrinking :-)
> diff --git a/src/include/port/aix.h b/src/include/port/aix.h
> new file mode 100644
> index 00000000000..7d08480c8c0
> --- /dev/null
> +++ b/src/include/port/aix.h
> @@ -0,0 +1,4 @@
> +/*
> + * src/include/port/aix.h
> + */
> +
Useless.
> diff --git a/src/include/storage/s_lock.h b/src/include/storage/s_lock.h
> index 2f73f9fcf57..5da9b3acda4 100644
> --- a/src/include/storage/s_lock.h
> +++ b/src/include/storage/s_lock.h
> @@ -421,17 +421,15 @@ tas(volatile slock_t *lock)
> __asm__ __volatile__(
> " lwarx %0,0,%3,1 \n"
> " cmpwi %0,0 \n"
> -" bne 1f \n"
> +" bne $+16 \n" /* branch to li %1,1 */
> " addi %0,%0,1 \n"
> " stwcx. %0,0,%3 \n"
> -" beq 2f \n"
> -"1: \n"
> +" beq $+12 \n" /* branch to lwsync */
> " li %1,1 \n"
> -" b 3f \n"
> -"2: \n"
> +" b $+12 \n" /* branch to end of asm sequence */
> " lwsync \n"
> " li %1,0 \n"
> -"3: \n"
> +
> : "=&b"(_t), "=r"(_res), "+m"(*lock)
> : "r"(lock)
> : "memory", "cc");
Why is this change needed?
Yes, I know we've been over this many times already. I still don't
understand why it's needed. The onus is on you to explain it adequately,
in comments in the patch, so that I and others understand it. Or even
better, remove it if it's not necessary.
> diff --git a/src/makefiles/Makefile.aix b/src/makefiles/Makefile.aix
> new file mode 100644
> index 00000000000..d33918f91b9
> --- /dev/null
> +++ b/src/makefiles/Makefile.aix
> @@ -0,0 +1,39 @@
> +# MAKE_EXPORTS is required for svr4 loaders that want a file of
> +# symbol names to tell them what to export/import.
> +MAKE_EXPORTS= true
Oh this is interesting. I think MAKE_EXPORTS is actually a leftover that
I failed to remove when I removed the AIX support; it's not used on any
currently-supported platform.
> +# -blibpath must contain ALL directories where we should look for libraries
> +libpath := $(shell echo $(subst -L,:,$(filter -L/%,$(LDFLAGS))) | sed -e's/ //g'):/usr/lib:/lib
Is this still sensible on modern AIX systems? What happens if you leave
it out?
> +# when building with gcc, need to make sure that libgcc can be found
> +ifeq ($(GCC), yes)
> +libpath := $(libpath):$(dir $(shell gcc -print-libgcc-file-name))
> +endif
Same here. Still relevant? What happens if you leave it out?
> +# gcc needs to know it's building a shared lib, otherwise it'll not emit
> +# correct code / link to the right support libraries
> +ifeq ($(GCC), yes)
> +LDFLAGS_SL += -shared
> +endif
On other platforms, we have this in LINK.shared, see src/Makefile.shlib.
Should we do the same on AIX; if not, why not?
> +# env var name to use in place of LD_LIBRARY_PATH
> +ld_library_path_var = LIBPATH
Does AIX have LD_LIBRARY_PATH? This suggests that it does:
https://www.ibm.com/support/pages/aix-libpath-recommendations
What's the difference between LIBPATH and LD_LIBRARY_PATH? Why prefer
LIBPATH?
Separately from the remaining issues with this patch, I still feel
pretty reluctant with accepting this, because if I commit this patch,
I'm on the hook to keep it working. I do regularly commit stuff that I'm
not personally that interested in, but a new port is different:
If something breaks on AIX, I have no means (or interest!) in debugging
it. That means that I need to be pretty confident that there are others
who are interested and invested in keeping working, take ownership, will
help to debug problems in a timely fashion, and can submit high-quality
fixes. I am not seeing that.
I also notice that all the AIX systems we have in the buildfarm are still:
- maintained by Noah, who - correct me if I'm wrong - is not
particularly interested in AIX or keen on maintaining them
- are on AIX 7.1.5, which I believe is already end-of-line
- running on power7 hardware which is 15 years old.
That's not very reassuring.
--
Heikki Linnakangas
Neon (https://neon.tech)
view thread (29+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: AIX support
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox