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: min/max builtins trigger mayCall ICE #60582

Closed
dmitshur opened this issue Jun 2, 2023 · 3 comments
Closed

cmd/compile: min/max builtins trigger mayCall ICE #60582

dmitshur opened this issue Jun 2, 2023 · 3 comments
Assignees
@cuonglm
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Milestone
Go1.21

Comments

@dmitshur
Copy link
Contributor

dmitshur commented Jun 2, 2023 •
edited

Tested with the most recent Go tip version:

$ go version
go version devel go1.21-6260049fa2 Fri Jun 2 19:54:05 2023 +0000 darwin/arm64

$ cd $(mktemp -d) && go mod init example && cat <<EOF > main.go
package main

import "fmt"

func main() {
	a, b := 5, 7
	fmt.Println(min(a, b))
}
EOF

$ go build
# example
./main.go:7:17: internal compiler error: mayCall 
.   MIN int tc(1) # main.go:7:17
.   MIN-Args
.   .   NAME-main.a esc(no) Class:PAUTO Offset:0 OnStack Used int tc(1) # main.go:6:2
.   .   NAME-main.b esc(no) Class:PAUTO Offset:0 OnStack Used int tc(1) # main.go:6:5

goroutine 1 [running]:
runtime/debug.Stack()
	/Users/gopher/gotip/src/runtime/debug/stack.go:24 +0x64
cmd/compile/internal/base.FatalfAt({0x1926c0?, 0x140?}, {0x102c2a06b, 0xb}, {0x14000453c38, 0x1, 0x1})
	/Users/gopher/gotip/src/cmd/compile/internal/base/print.go:230 +0x1fc
cmd/compile/internal/walk.mayCall.func2({0x102e62018, 0x1400011e960})
	/Users/gopher/gotip/src/cmd/compile/internal/walk/walk.go:308 +0x408
cmd/compile/internal/ir.Any.func1({0x102e62018, 0x1400011e960})
	/Users/gopher/gotip/src/cmd/compile/internal/ir/visit.go:130 +0x38
cmd/compile/internal/ir.(*ConvExpr).doChildren(0x140001926c0, 0x1400069a318)
	/Users/gopher/gotip/src/cmd/compile/internal/ir/node_gen.go:503 +0xc0
cmd/compile/internal/ir.DoChildren(...)
	/Users/gopher/gotip/src/cmd/compile/internal/ir/visit.go:94
cmd/compile/internal/ir.Any.func1({0x102e623d8, 0x140001926c0})
	/Users/gopher/gotip/src/cmd/compile/internal/ir/visit.go:130 +0x68
cmd/compile/internal/ir.Any({0x102e623d8, 0x140001926c0}, 0x14000660f20)
	/Users/gopher/gotip/src/cmd/compile/internal/ir/visit.go:132 +0xb0
cmd/compile/internal/walk.mayCall({0x102e623d8, 0x140001926c0})
	/Users/gopher/gotip/src/cmd/compile/internal/walk/walk.go:299 +0x78
cmd/compile/internal/walk.walkCall1(0x1400011ed20, 0x14000454ac0)
	/Users/gopher/gotip/src/cmd/compile/internal/walk/expr.go:619 +0x2bc
cmd/compile/internal/walk.walkCall(0x1400011ed20, 0x10249c380?)
	/Users/gopher/gotip/src/cmd/compile/internal/walk/expr.go:589 +0xb00
cmd/compile/internal/walk.walkExpr1({0x102e62018, 0x1400011ed20}, 0x1400011ed20?)
	/Users/gopher/gotip/src/cmd/compile/internal/walk/expr.go:189 +0x3a8
cmd/compile/internal/walk.walkExpr({0x102e62018, 0x1400011ed20}, 0x14000454ac0)
	/Users/gopher/gotip/src/cmd/compile/internal/walk/expr.go:56 +0x25c
cmd/compile/internal/walk.dataWord(0x14000192240, 0x14000454ac0)
	/Users/gopher/gotip/src/cmd/compile/internal/walk/convert.go:211 +0xc84
cmd/compile/internal/walk.walkConvInterface(0x14000192240, 0x14000454ac0)
	/Users/gopher/gotip/src/cmd/compile/internal/walk/convert.go:60 +0xb44
cmd/compile/internal/walk.walkExpr1({0x102e623d8, 0x14000192240}, 0x14000192240?)
	/Users/gopher/gotip/src/cmd/compile/internal/walk/expr.go:224 +0x470
cmd/compile/internal/walk.walkExpr({0x102e623d8, 0x14000192240}, 0x14000454ac0)
	/Users/gopher/gotip/src/cmd/compile/internal/walk/expr.go:56 +0x25c
cmd/compile/internal/walk.walkAssign(0x14000454ac0, {0x102e63158, 0x14000697680})
	/Users/gopher/gotip/src/cmd/compile/internal/walk/assign.go:72 +0xae0
cmd/compile/internal/walk.walkExpr1({0x102e63158, 0x14000697680}, 0x14000697680?)
	/Users/gopher/gotip/src/cmd/compile/internal/walk/expr.go:192 +0x37c
cmd/compile/internal/walk.walkExpr({0x102e63158, 0x14000697680}, 0x14000454ac0)
	/Users/gopher/gotip/src/cmd/compile/internal/walk/expr.go:56 +0x25c
cmd/compile/internal/walk.walkStmt({0x102e63158, 0x14000697680?})
	/Users/gopher/gotip/src/cmd/compile/internal/walk/stmt.go:59 +0x728
cmd/compile/internal/walk.walkStmtList(...)
	/Users/gopher/gotip/src/cmd/compile/internal/walk/stmt.go:175
cmd/compile/internal/walk.walkStmt({0x102e62cd8, 0x14000684b40?})
	/Users/gopher/gotip/src/cmd/compile/internal/walk/stmt.go:98 +0x1054
cmd/compile/internal/walk.appendWalkStmt(0x140004553a0, {0x102e62cd8, 0x14000684b40})
	/Users/gopher/gotip/src/cmd/compile/internal/walk/walk.go:258 +0x70
cmd/compile/internal/walk.slicelit(0x0, 0x14000176a00, {0x102e61d18?, 0x14000665ad0}, 0x1?)
	/Users/gopher/gotip/src/cmd/compile/internal/walk/complit.go:409 +0x9b0
cmd/compile/internal/walk.anylit({0x102e62b58, 0x14000176a00}, {0x102e61d18?, 0x14000665ad0}, 0x14000454fa8?)
	/Users/gopher/gotip/src/cmd/compile/internal/walk/complit.go:600 +0x448
cmd/compile/internal/walk.oaslit(...)
	/Users/gopher/gotip/src/cmd/compile/internal/walk/complit.go:649
cmd/compile/internal/walk.walkAssign(0x140004553a0, {0x102e63158, 0x140006974f0})
	/Users/gopher/gotip/src/cmd/compile/internal/walk/assign.go:57 +0x74c
cmd/compile/internal/walk.walkExpr1({0x102e63158, 0x140006974f0}, 0x140006974f0?)
	/Users/gopher/gotip/src/cmd/compile/internal/walk/expr.go:192 +0x37c
cmd/compile/internal/walk.walkExpr({0x102e63158, 0x140006974f0}, 0x140004553a0)
	/Users/gopher/gotip/src/cmd/compile/internal/walk/expr.go:56 +0x25c
cmd/compile/internal/walk.appendWalkStmt(...)
	/Users/gopher/gotip/src/cmd/compile/internal/walk/walk.go:256
cmd/compile/internal/walk.ascompatee(0x16, {0x14000660d00?, 0x1, 0x14000684940?}, {0x14000660d40?, 0x1, 0x102e61d18?})
	/Users/gopher/gotip/src/cmd/compile/internal/walk/assign.go:375 +0x64c
cmd/compile/internal/walk.walkAssignList(0x140004557f0, 0x14000699380)
	/Users/gopher/gotip/src/cmd/compile/internal/walk/assign.go:147 +0x12c
cmd/compile/internal/walk.walkExpr1({0x102e62e58, 0x14000699380}, 0x14000699380?)
	/Users/gopher/gotip/src/cmd/compile/internal/walk/expr.go:196 +0x2b0
cmd/compile/internal/walk.walkExpr({0x102e62e58, 0x14000699380}, 0x140004557f0)
	/Users/gopher/gotip/src/cmd/compile/internal/walk/expr.go:56 +0x25c
cmd/compile/internal/walk.walkStmt({0x102e62e58, 0x14000699380?})
	/Users/gopher/gotip/src/cmd/compile/internal/walk/stmt.go:59 +0x728
cmd/compile/internal/walk.walkStmtList(...)
	/Users/gopher/gotip/src/cmd/compile/internal/walk/stmt.go:175
cmd/compile/internal/walk.Walk(0x1400042de40)
	/Users/gopher/gotip/src/cmd/compile/internal/walk/walk.go:43 +0x15c
cmd/compile/internal/gc.prepareFunc(0x1400042de40)
	/Users/gopher/gotip/src/cmd/compile/internal/gc/compile.go:105 +0x104
cmd/compile/internal/gc.enqueueFunc(0x1400042de40)
	/Users/gopher/gotip/src/cmd/compile/internal/gc/compile.go:71 +0x2b4
cmd/compile/internal/gc.Main(0x102e56530)
	/Users/gopher/gotip/src/cmd/compile/internal/gc/main.go:338 +0x131c
main.main()
	/Users/gopher/gotip/src/cmd/compile/main.go:57 +0x110

$ echo $?
1

(https://go.dev/play/p/Td1MrT9ErxG?v=gotip)

Notably, simplifying further in the following ways causes it to stop reproducing:

  • using println instead of fmt.Println
  • assigning result of min to _ instead of fmt.Println
  • assigning result of min to another variable and fmt.Printlning that
  • using constants instead of variables for a, b

(Rebuilding cmd/compile without GOROOT/src/cmd/compile/default.pgo doesn't stop it from reproducing.)

CC @golang/compiler.
@dmitshur dmitshur added release-blocker compiler/runtime Issues related to the Go compiler and/or runtime. labels Jun 2, 2023
@dmitshur dmitshur added this to the Go1.21 milestone Jun 2, 2023
@gopherbot
Copy link

Change https://go.dev/cl/500575 mentions this issue: cmd/compile: allow ir.OMIN/ir.OMAX in mayCall

@dmitshur dmitshur added the NeedsFix The path to resolution is known, but the work has not been done. label Jun 3, 2023
@cuonglm
Copy link
Member

cuonglm commented Jun 3, 2023

The problem is that in fmt.Println(min(a, b)), min(a, b) is passed to runtime.convT64 function, so it will be evaluated by mayCall.

@cuonglm cuonglm mentioned this issue Jun 5, 2023
Closed
@gopherbot
Copy link

Change https://go.dev/cl/506855 mentions this issue: cmd/compile: handle min/max correctly in mayCall

gopherbot pushed a commit that referenced this issue Jun 28, 2023
@cuonglm @gopherbot
cmd/compile: handle min/max correctly in mayCall
8b5fe59 
CL 500575 changed mayCall to return "false" for min/max builtin.

However, with string or float, min/max requires runtime call, so mayCall
should return true instead. This's probably not a big problem, because
CL 506115 makes order pass handle min/max correctly. But it's still
better to do it the right way.

Updates #60582

Change-Id: I9779ca62bebd0f95e52ad5fa55b9160dc35b33aa
Reviewed-on: https://go-review.googlesource.com/c/go/+/506855
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
bradfitz pushed a commit to tailscale/go that referenced this issue Jul 15, 2023
@cuonglm @bradfitz
cmd/compile: handle min/max correctly in mayCall
6a2df45 
CL 500575 changed mayCall to return "false" for min/max builtin.

However, with string or float, min/max requires runtime call, so mayCall
should return true instead. This's probably not a big problem, because
CL 506115 makes order pass handle min/max correctly. But it's still
better to do it the right way.

Updates golang#60582

Change-Id: I9779ca62bebd0f95e52ad5fa55b9160dc35b33aa
Reviewed-on: https://go-review.googlesource.com/c/go/+/506855
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@cuonglm cuonglm

Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Projects
None yet
Milestone
Go1.21
Development

No branches or pull requests
3 participants
@dmitshur @cuonglm @gopherbot