Skip to content

Commit

Permalink
GH-36052: [Go][Parquet] Cross build failures for 386 (#36066)
Browse files Browse the repository at this point in the history
### 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 <zotthewizard@gmail.com>
Signed-off-by: Matt Topol <zotthewizard@gmail.com>
  • Loading branch information
zeroshade committed Jun 14, 2023
1 parent 340f6c8 commit 4a53764
Show file tree
Hide file tree
Showing 10 changed files with 39 additions and 109 deletions.
26 changes: 26 additions & 0 deletions .github/workflows/go.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
Expand Down Expand Up @@ -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
Expand Down
5 changes: 1 addition & 4 deletions go/parquet/internal/utils/bit_packing_amd64.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 7 additions & 9 deletions go/parquet/internal/utils/bit_packing_arm64.go
Original file line number Diff line number Diff line change
Expand Up @@ -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, ",")

Expand All @@ -53,4 +52,3 @@ func init() {
unpack32 = unpack32Default
}
}

2 changes: 2 additions & 0 deletions go/parquet/internal/utils/bit_packing_default.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
23 changes: 0 additions & 23 deletions go/parquet/internal/utils/bit_packing_noasm.go

This file was deleted.

23 changes: 0 additions & 23 deletions go/parquet/internal/utils/bit_packing_ppc64le.go

This file was deleted.

23 changes: 0 additions & 23 deletions go/parquet/internal/utils/bit_packing_s390x.go

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
25 changes: 0 additions & 25 deletions go/parquet/internal/utils/unpack_bool_ppc64le.go

This file was deleted.

2 changes: 1 addition & 1 deletion go/parquet/metadata/statistics.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 4a53764

Please sign in to comment.