Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

cmd/compile: constants in -S output for arm64 have extra high bits set #53551

Closed
randall77 opened this issue Jun 25, 2022 · 2 comments
Closed

cmd/compile: constants in -S output for arm64 have extra high bits set #53551

randall77 opened this issue Jun 25, 2022 · 2 comments
Assignees
@erifan
Labels
FrozenDueToAge
Milestone
Go1.19

Comments

@randall77
Copy link
Contributor 

package main

func f(x *int8) bool {
	return *x <= 43
}

Compiling this program with -S for arm64 has the line:

CMPW	$184683593771, R1

It should be

CMPW	$43, R1

Note that 184683593771 = 43<<32 + 43.
Go 1.16 is ok, this started in Go 1.17. Bisect points to https://go-review.googlesource.com/c/go/+/289649 . It looks maybe intentional? Not sure, but definitely weird.

The actual machine code looks correct. Builders are passing, and go tool objdump shows the right disassembly. It's just the -S output that appears broken.

@cherrymui @erifan
@randall77 randall77 added this to the Go1.19 milestone Jun 25, 2022
@erifan
Copy link

erifan commented Jun 26, 2022

This problem has existed for a long time, but CL 289649 exposed it. The root cause is that we modified this constant operand in progedit by copying the lower 32 bits to the upper 32 bits.

We modify this constant just to determine whether it is a C_BITCON class and to encode it as a C_BITCON. So maybe we can do this modification (copy the lower 32 bits to the upper 32 bits) just before these two operations (con32class and bitconEncode ), and assign it to a temporary variable, so that the print result will not be modified.

Thanks for reporting this issue, I'll send a fix.

@erifan erifan self-assigned this Jun 26, 2022
@gopherbot
Copy link

Change https://go.dev/cl/414374 mentions this issue: cmd/interna/obj/arm64: fix BITCON constant printing error

jproberts pushed a commit to jproberts/go that referenced this issue Aug 10, 2022
@jproberts
cmd/internal/obj/arm64: fix BITCON constant printing error
bea3155 
For some 32-bit instructions whose first operand is a constant, we
copy the lower 32 bits of the constant into the upper 32 bits in progedit,
which leads to the wrong value being printed in -S output.

The purpose of this is that we don't need to distinguish between 32-bit
and 64-bit constants when checking C_BITCON, this CL puts the modified
value in a temporary variable, so that the constant operand of the
instruction will not be modified.

Fixes golang#53551

Change-Id: I40ee9223b4187bff1c0a1bab7eb508fcb30325f9
Reviewed-on: https://go-review.googlesource.com/c/go/+/414374
Run-TryBot: Eric Fang <eric.fang@arm.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
@golang golang locked and limited conversation to collaborators Jun 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@erifan erifan

Labels
FrozenDueToAge
Projects
None yet
Milestone
Go1.19
Development

No branches or pull requests
3 participants
@randall77 @gopherbot @erifan