You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by ze...@apache.org on 2023/06/14 17:09:11 UTC
[arrow] branch main updated: GH-36052: [Go][Parquet] Cross build failures for 386 (#36066)
This is an automated email from the ASF dual-hosted git repository.
zeroshade pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/main by this push:
new 4a53764594 GH-36052: [Go][Parquet] Cross build failures for 386 (#36066)
4a53764594 is described below
commit 4a537645944d46faa688e8397fb5f85ed0d952f2
Author: Matt Topol <zo...@gmail.com>
AuthorDate: Wed Jun 14 13:09:03 2023 -0400
GH-36052: [Go][Parquet] Cross build failures for 386 (#36066)
### Rationale for this change
It looks like #35133 only addressed 32-bit build failures in the Arrow library but didn't address the Parquet library failing to build on 32-bit systems. So this fixes building the Go Parquet library for GOARCH=386
### What changes are included in this PR?
Simplify and clean up build tags and specialized asm in Go Parquet library so that we don't need to keep adding new files explicitly for architectures that we didn't generate optimized assembly for. So we set up the appropriate defaults so that those architectures will default to the pure go implementation properly.
Also fixes the usage of `math.Uint32` so that it isn't seen as an untyped int constant which would overflow on 32-bit architectures.
### Are these changes tested?
Yes, added a small workflow that builds the Go arrow and parquet libraries with `GOARCH=386`.
* Closes: #36052
Authored-by: Matt Topol <zo...@gmail.com>
Signed-off-by: Matt Topol <zo...@gmail.com>
---
.github/workflows/go.yml | 26 ++++++++++++++++++++++
go/parquet/internal/utils/bit_packing_amd64.go | 5 +----
go/parquet/internal/utils/bit_packing_arm64.go | 16 ++++++-------
go/parquet/internal/utils/bit_packing_default.go | 2 ++
go/parquet/internal/utils/bit_packing_noasm.go | 23 -------------------
go/parquet/internal/utils/bit_packing_ppc64le.go | 23 -------------------
go/parquet/internal/utils/bit_packing_s390x.go | 23 -------------------
...unpack_bool_s390x.go => unpack_bool_default.go} | 3 ++-
go/parquet/internal/utils/unpack_bool_ppc64le.go | 25 ---------------------
go/parquet/metadata/statistics.go | 2 +-
10 files changed, 39 insertions(+), 109 deletions(-)
diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml
index 5edc579e69..89c02f5ad7 100644
--- a/.github/workflows/go.yml
+++ b/.github/workflows/go.yml
@@ -65,6 +65,11 @@ jobs:
go: 1.17
staticcheck: v0.2.2
runs-on: ["self-hosted", "arm", "linux"]
+ - arch-label: ARM64
+ arch: arm64v8
+ go: 1.18
+ staticcheck: v0.3.3
+ runs-on: ["self-hosted", "arm", "linux"]
env:
ARCH: ${{ matrix.arch }}
GO: ${{ matrix.go }}
@@ -122,6 +127,27 @@ jobs:
python3 -m pip install benchadapt@git+https://github.com/conbench/conbench.git@main#subdirectory=benchadapt/python
python3 ci/scripts/go_bench_adapt.py
+ build386:
+ name: Go Cross-build for 386
+ runs-on: ubuntu-latest
+ if: ${{ !contains(github.event.pull_request.title, 'WIP') }}
+ timeout-minutes: 20
+ steps:
+ - name: Checkout Arrow
+ uses: actions/checkout@v3
+ with:
+ fetch-depth: 0
+ - name: Install Go
+ uses: actions/setup-go@v3
+ with:
+ go-version: 1.18
+ cache: true
+ cache-dependency-path: go/go.sum
+ - name: Run build
+ run: |
+ cd go
+ GOARCH=386 go build ./...
+
docker_cgo:
name: AMD64 Debian 11 Go ${{ matrix.go }} - CGO
runs-on: ubuntu-latest
diff --git a/go/parquet/internal/utils/bit_packing_amd64.go b/go/parquet/internal/utils/bit_packing_amd64.go
index dbbb5159c1..72702578c1 100644
--- a/go/parquet/internal/utils/bit_packing_amd64.go
+++ b/go/parquet/internal/utils/bit_packing_amd64.go
@@ -14,18 +14,15 @@
// See the License for the specific language governing permissions and
// limitations under the License.
+//go:build !noasm
// +build !noasm
package utils
import (
- "io"
-
"golang.org/x/sys/cpu"
)
-var unpack32 func(io.Reader, []uint32, int) int
-
func init() {
if cpu.X86.HasAVX2 {
unpack32 = unpack32Avx2
diff --git a/go/parquet/internal/utils/bit_packing_arm64.go b/go/parquet/internal/utils/bit_packing_arm64.go
index 7e9b39ffe1..61eead0263 100644
--- a/go/parquet/internal/utils/bit_packing_arm64.go
+++ b/go/parquet/internal/utils/bit_packing_arm64.go
@@ -14,24 +14,23 @@
// See the License for the specific language governing permissions and
// limitations under the License.
+//go:build !noasm
// +build !noasm
package utils
import (
- "io"
"os"
"strings"
-)
-import "golang.org/x/sys/cpu"
-var unpack32 func(io.Reader, []uint32, int) int = unpack32Default
+ "golang.org/x/sys/cpu"
+)
func init() {
- cpu.ARM64.HasASIMD = false
- cpu.ARM64.HasAES = false
- cpu.ARM64.HasPMULL = false
- // Added ability to enable extension via environment:
+ cpu.ARM64.HasASIMD = false
+ cpu.ARM64.HasAES = false
+ cpu.ARM64.HasPMULL = false
+ // Added ability to enable extension via environment:
if ext, ok := os.LookupEnv("ARM_ENABLE_EXT"); ok {
exts := strings.Split(ext, ",")
@@ -53,4 +52,3 @@ func init() {
unpack32 = unpack32Default
}
}
-
diff --git a/go/parquet/internal/utils/bit_packing_default.go b/go/parquet/internal/utils/bit_packing_default.go
index 64f4fecd2b..fa814f6375 100644
--- a/go/parquet/internal/utils/bit_packing_default.go
+++ b/go/parquet/internal/utils/bit_packing_default.go
@@ -21,6 +21,8 @@ import (
"io"
)
+var unpack32 func(io.Reader, []uint32, int) int = unpack32Default
+
type unpackFunc func(in io.Reader, out []uint32)
func unpack1_32(in io.Reader, out []uint32) {
diff --git a/go/parquet/internal/utils/bit_packing_noasm.go b/go/parquet/internal/utils/bit_packing_noasm.go
deleted file mode 100644
index a5e03814cf..0000000000
--- a/go/parquet/internal/utils/bit_packing_noasm.go
+++ /dev/null
@@ -1,23 +0,0 @@
-// Licensed to the Apache Software Foundation (ASF) under one
-// or more contributor license agreements. See the NOTICE file
-// distributed with this work for additional information
-// regarding copyright ownership. The ASF licenses this file
-// to you under the Apache License, Version 2.0 (the
-// "License"); you may not use this file except in compliance
-// with the License. You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-// +build noasm
-
-package utils
-
-import "io"
-
-var unpack32 func(io.Reader, []uint32, int) int = unpack32Default
diff --git a/go/parquet/internal/utils/bit_packing_ppc64le.go b/go/parquet/internal/utils/bit_packing_ppc64le.go
deleted file mode 100644
index 58f869c3f5..0000000000
--- a/go/parquet/internal/utils/bit_packing_ppc64le.go
+++ /dev/null
@@ -1,23 +0,0 @@
-// Licensed to the Apache Software Foundation (ASF) under one
-// or more contributor license agreements. See the NOTICE file
-// distributed with this work for additional information
-// regarding copyright ownership. The ASF licenses this file
-// to you under the Apache License, Version 2.0 (the
-// "License"); you may not use this file except in compliance
-// with the License. You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-// +build !noasm
-
-package utils
-
-import "io"
-
-var unpack32 func(io.Reader, []uint32, int) int = unpack32Default
diff --git a/go/parquet/internal/utils/bit_packing_s390x.go b/go/parquet/internal/utils/bit_packing_s390x.go
deleted file mode 100644
index 58f869c3f5..0000000000
--- a/go/parquet/internal/utils/bit_packing_s390x.go
+++ /dev/null
@@ -1,23 +0,0 @@
-// Licensed to the Apache Software Foundation (ASF) under one
-// or more contributor license agreements. See the NOTICE file
-// distributed with this work for additional information
-// regarding copyright ownership. The ASF licenses this file
-// to you under the Apache License, Version 2.0 (the
-// "License"); you may not use this file except in compliance
-// with the License. You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-// +build !noasm
-
-package utils
-
-import "io"
-
-var unpack32 func(io.Reader, []uint32, int) int = unpack32Default
diff --git a/go/parquet/internal/utils/unpack_bool_s390x.go b/go/parquet/internal/utils/unpack_bool_default.go
similarity index 93%
rename from go/parquet/internal/utils/unpack_bool_s390x.go
rename to go/parquet/internal/utils/unpack_bool_default.go
index d833c2b9d6..6aee1c8d00 100644
--- a/go/parquet/internal/utils/unpack_bool_s390x.go
+++ b/go/parquet/internal/utils/unpack_bool_default.go
@@ -14,7 +14,8 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-// +build !noasm
+//go:build !noasm && !amd64 && !arm64
+// +build !noasm,!amd64,!arm64
package utils
diff --git a/go/parquet/internal/utils/unpack_bool_ppc64le.go b/go/parquet/internal/utils/unpack_bool_ppc64le.go
deleted file mode 100644
index d833c2b9d6..0000000000
--- a/go/parquet/internal/utils/unpack_bool_ppc64le.go
+++ /dev/null
@@ -1,25 +0,0 @@
-// Licensed to the Apache Software Foundation (ASF) under one
-// or more contributor license agreements. See the NOTICE file
-// distributed with this work for additional information
-// regarding copyright ownership. The ASF licenses this file
-// to you under the Apache License, Version 2.0 (the
-// "License"); you may not use this file except in compliance
-// with the License. You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-// +build !noasm
-
-package utils
-
-// BytesToBools when built with the noasm tag will direct to the pure go implementation
-// for converting a bitmap to a slice of bools
-func BytesToBools(in []byte, out []bool) {
- bytesToBoolsGo(in, out)
-}
diff --git a/go/parquet/metadata/statistics.go b/go/parquet/metadata/statistics.go
index 56328ba814..11ec0b6207 100644
--- a/go/parquet/metadata/statistics.go
+++ b/go/parquet/metadata/statistics.go
@@ -340,7 +340,7 @@ func (BooleanStatistics) defaultMin() bool { return true }
func (BooleanStatistics) defaultMax() bool { return false }
func (s *Int32Statistics) defaultMin() int32 {
if s.order == schema.SortUNSIGNED {
- val := math.MaxUint32
+ val := uint32(math.MaxUint32)
return int32(val)
}
return math.MaxInt32