Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1nh6rr-0006vQ-Jl for pgsql-hackers@arkaria.postgresql.org; Wed, 20 Apr 2022 09:43:23 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1nh6rp-00044Y-5K for pgsql-hackers@arkaria.postgresql.org; Wed, 20 Apr 2022 09:43:21 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1nh6ro-00040A-MH for pgsql-hackers@lists.postgresql.org; Wed, 20 Apr 2022 09:43:20 +0000 Received: from mail-qt1-x830.google.com ([2607:f8b0:4864:20::830]) by makus.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.92) (envelope-from ) id 1nh6rm-0006lL-1Y for pgsql-hackers@lists.postgresql.org; Wed, 20 Apr 2022 09:43:19 +0000 Received: by mail-qt1-x830.google.com with SMTP id f14so575623qtq.1 for ; Wed, 20 Apr 2022 02:43:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=na/8C7qSRbBa202ZmwNVq/ZrjK5ykz5JVF5QwMaCYbU=; b=e91wmttGYnFfxjo9jTmTDIqy1ZW+z9iMsy7h+C6qJZoJi/L5DzbPEAJVy/K41dbVZh 5ei1b+e+Gq7KSGwPjiUcDqDlvZ92t3ZRMUHCvhRmIcJJSYuKbHXXLTHz03sfiocnI7Gl 5Eg+0cnUPfavHzCfV+PcJi2jKtc0/yYHamSUKBx0e/7e5wKf9FpyW+PhpXpeGxg3we/w SXLxDf4wKUkoVxo+G4wGGt7mGR1SlcKzzA5F9iOAQf3Ncon+doLrogyY5mf0k9PWv+ev 8ROQBgjOC/PcEN1DaMDD0UdfPfXzjG9k/tfqjiLlOT2qO/dF/zLJIVYIS9k6IMB6sxo2 XrfA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=na/8C7qSRbBa202ZmwNVq/ZrjK5ykz5JVF5QwMaCYbU=; b=JcBpA9/PRlv3bGZ40c7qhBgq5xS4MqGSIo+ae7B+bij/eXz11bfaLtKpPBvJxIQpUu 7BDlpNox2299VAoKWuvSCJhenDPx9/XeKyhC5wtyOx7XzsiO5Ud4KXfcQ0Wjqt+8Yia3 X7CZSDGPAQ9d1BXphVcsjnZ3ABohsvPXbRM29EOk5fDZfVkR0WeF/mU8iG9YhmuOqkyb pj2xf8Is3L3V+YGq+D4lg5DG9bS6AuZhmWv+gJ2bObDChxO40H4KQwBoRILHWaDXZLMH FeAHkhJuImpeRSCmB00L75bGqIMOEQF5Cw1VhhKHlDZwuOOR5E0voJrbAyFL7rvFNjwH FFnw== X-Gm-Message-State: AOAM533tU3bUubv9qWc/Lx9Mu/uMRXrYLDL2MhNUpgERZYs1t+qKBu+9 wKleY4BAuEJ6HwhL/lLGbnH6D4bKzGdvPI0hs/gmIw== X-Google-Smtp-Source: ABdhPJygUxgwnnKL0pb7TEoeuzP8/ABkCBBVNJzat1bWBNx90q7kn3gxz52dqXhiCHVoXH7WKw8YxsPdgynKSwS4k3w= X-Received: by 2002:ac8:5b93:0:b0:2e1:ecde:e1e2 with SMTP id a19-20020ac85b93000000b002e1ecdee1e2mr13126251qta.304.1650447797051; Wed, 20 Apr 2022 02:43:17 -0700 (PDT) MIME-Version: 1.0 References: <20220322103011.i6z2tuj4hle23wgx@jrouhaud> <20220324011531.va6z5pst5ygtu6zu@alap3.anarazel.de> <3278490.1649338843@sss.pgh.pa.us> <20220407172258.uj5lgd6k25sngeno@alap3.anarazel.de> <3385302.1649353369@sss.pgh.pa.us> <20220407175359.h7gzgmmnbicq75d6@alap3.anarazel.de> In-Reply-To: From: Niyas Sait Date: Wed, 20 Apr 2022 10:43:06 +0100 Message-ID: Subject: Re: [PATCH] Add native windows on arm64 support To: Michael Paquier Cc: Andres Freund , Tom Lane , Thomas Munro , Julien Rouhaud , PostgreSQL Hackers Content-Type: multipart/alternative; boundary="000000000000a9f57405dd12d2bf" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --000000000000a9f57405dd12d2bf Content-Type: text/plain; charset="UTF-8" > Have you tested with the amount of coverage provided by vcregress.pl? I built and ran the relevant tests with the help of run_build.pl. I think following tests are executed - check, contribcheck, ecpgcheck, installcheck, isolationcheck, modulescheck, and upgradecheck. > Another thing I was wondering about is if it would be possible to have > this option in Travis, but that does not seem to be the case: > https://docs.travis-ci.com/user/reference/windows/#windows-version Yes, I think Travis doesn't yet support Windows/Arm64 platform. > + if ($solution->{platform} eq 'ARM64') { > + push(@pgportfiles, 'pg_crc32c_armv8_choose.c'); > + push(@pgportfiles, 'pg_crc32c_armv8.c'); > + } else { > + push(@pgportfiles, 'pg_crc32c_sse42_choose.c'); > + push(@pgportfiles, 'pg_crc32c_sse42.c'); > + } > > +++ b/src/port/pg_crc32c_armv8.c > +#ifndef _MSC_VER > #include > +#endif > [ ... ] > +#ifdef _M_ARM64 > + /* > + * arm64 way of hinting processor for spin loops optimisations > + * ref: https://community.arm.com/support-forums/f/infrastructure-solutions-forum/48654/ssetoneon-faq > + */ > + __isb(_ARM64_BARRIER_SY); > +#else > I think that such extra optimizations had better be in a separate > patch, and we should focus on getting the build done first. > This would mean that a basic patch could be done with just the changes > for gendef.pl, and the first part of the changes inMSBuildProject.pm. > Would that not be enough? I cannot build without any of the above changes. Nothing specific for optimization is added to this patch. > + # arm64 linker only supports dynamic base address > + my $cfgrandbaseaddress = $self->{platform} eq 'ARM64' ? 'true' : 'false'; > This issue is still lying around, and you may have been lucky. Would > there be any issues to remove this change to get a basic support in? > As mentioned upthread, there is a long history of Postgres with ASLR. MSVC linker doesn't allow non-random base addresses for Arm64 platforms. It is needed for basic support. Niyas --000000000000a9f57405dd12d2bf Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
> Have you tested with the amount of c= overage provided by=C2=A0vcregress.pl?

<= div>
I built and ran the relevant tests wi= th the help of run_build.pl.

<= div>I think following tests are executed - check, contribcheck, ecpgcheck, = installcheck, isolationcheck,=C2=A0modulescheck, and upgradecheck.
> Another thing I was wondering about is if it would be= possible to have
> this option in Travis, but that does not seem to = be the case:
>=C2=A0https://= docs.travis-ci.com/user/reference/windows/#windows-version

Yes, I= think Travis doesn't yet support Windows/Arm64 platform.
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if= ($solution->{platform} eq 'ARM64') {
>=C2=A0+=C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0pu= sh(@pgportfiles, 'pg_crc32c_armv8_choose.c');
>=C2=A0+=C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0push(@pgportfiles, 'pg_crc32c_armv8.c');
>=C2=A0+=C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0} else {
>=C2=A0+=C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0push(@pgportfiles, 'pg_crc32c_sse42_choose.c');
>=C2=A0= +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0push(@pgportfiles, 'pg_crc32c_sse42.c');
>=C2=A0+= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0}
>=C2=A0
&= gt;=C2=A0+++ b/src/port/pg_crc32c_armv8.c
>=C2=A0+#ifndef _MSC_VER>=C2=A0=C2=A0#include <arm_acle.h>
>=C2=A0+#endif
>= =C2=A0[ ... ]
>=C2=A0+#ifdef _M_ARM64
>=C2=A0+=C2=A0 =C2=A0/*>=C2=A0+=C2=A0 =C2=A0 * arm64 way of hinting processor for spin loops = optimisations
>=C2=A0+=C2=A0 =C2=A0 * ref:=C2=A0https://community.arm.com/su= pport-forums/f/infrastructure-solutions-forum/48654/ssetoneon-faq
&g= t;=C2=A0+=C2=A0 =C2=A0 */
>=C2=A0+=C2=A0 =C2=A0__isb(_ARM64_BARRIER_S= Y);
>=C2=A0+#else
>=C2=A0I think that such extra optimizations = had better be in a separate
>=C2=A0patch, and we should focus on gett= ing the build done first.

> This would mean= that a basic patch could be done with just the changes
> for=C2=A0gendef.pl<= /a>, and the first part of the changes inMSBuildProject.pm.
> Would t= hat not be enough?

I cannot build without any = of the above changes. Nothing specific for optimization
is added = to this patch.

> +=C2=A0 =C2=A0# arm64 linker o= nly supports dynamic base address
> +=C2=A0 =C2=A0my $cfgrandbaseaddr= ess =3D $self->{platform} eq 'ARM64' ? 'true' : 'fal= se';
> This issue is still lying around, and you may have been lu= cky.=C2=A0 Would
> there be any issues to remove this change to get a= basic support in?
> As mentioned upthread, there is a long history o= f Postgres with ASLR.

MSVC linker doesn't = allow non-random base addresses for Arm64 platforms.
It is needed= for basic support.

Niyas
--000000000000a9f57405dd12d2bf--