golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.99k stars 17.67k forks source link

x/tools/go/analysis/passes/asmdecl: false error report for 16 byte move #39220

Open mmcloughlin opened 4 years ago

mmcloughlin commented 4 years ago

The following assembly

#include "textflag.h"

// func Shuffle(mask []byte, data []byte) [4]uint32
// Requires: SSE2, SSSE3
TEXT ·Shuffle(SB), NOSPLIT, $0-64
    MOVQ   mask_base+0(FP), AX
    MOVQ   data_base+24(FP), CX
    MOVOU  (CX), X0
    PSHUFB (AX), X0
    MOVOU  X0, ret+48(FP)
    RET

produces the following error report from asmdecl:

[amd64] Shuffle: invalid MOVOU of ret+48(FP); [4]uint32 is 16-byte value

Since MOVOU is a 128-bit move, this is a false positive.

The error is produced here: https://github.com/golang/tools/blob/73554e0f78058c37e5421bc48273a72400172221/go/analysis/passes/asmdecl/asmdecl.go#L781

Inspecting this code, it's clear that the move size detection does not handle sizes of 16 bytes and up.

andybons commented 4 years ago

@matloob

randall77 commented 4 years ago

Related CL (support for complex128): https://go-review.googlesource.com/c/tools/+/204537

mmcloughlin commented 4 years ago

Ah thank you @randall77. My attempt to avoid this linter error was to change my test case to 64-bits rather than 128-bit (the PR is mmcloughlin/avo#149).

// Code generated by command: go run asm.go -out issue145.s -stubs stub.go. DO NOT EDIT.

#include "textflag.h"

// func Halves(x uint64) [2]uint32
TEXT ·Halves(SB), NOSPLIT, $0-16
    MOVQ x+0(FP), AX
    MOVQ AX, ret+8(FP)
    RET

This gives the following error

[amd64] Halves: invalid MOVQ of ret+8(FP); [2]uint32 is 8-byte value

As I result I suspect that there are two aspects to this:

zchee commented 4 years ago

@randall77 @mmcloughlin sorry mentioned, I would like to know the current status.

randall77 commented 4 years ago

I know of no one actively working on this. Patches welcome (1.16 freeze is on us, so it would be for 1.17).

zchee commented 4 years ago

@randall77 Thanks, will do.