Skip to content

cmd/compile: remove GOEXPERIMENT=nounified #57410

Closed
@mdempsky

Description

@mdempsky

Tracking issue for removing GOEXPERIMENT=nounified for Go 1.21.

Activity

added
NeedsFixThe path to resolution is known, but the work has not been done.
compiler/runtimeIssues related to the Go compiler and/or runtime.
on Dec 20, 2022
added this to the Go1.21 milestone on Dec 20, 2022
self-assigned this
on Dec 20, 2022
gopherbot

gopherbot commented on Dec 20, 2022

@gopherbot
Contributor

Change https://go.dev/cl/458624 mentions this issue: cmd/compile/internal/types: remove more unused features

gopherbot

gopherbot commented on Dec 20, 2022

@gopherbot
Contributor

Change https://go.dev/cl/458623 mentions this issue: cmd/compile/internal/types: remove NewSignature's tparams parameter

gopherbot

gopherbot commented on Dec 20, 2022

@gopherbot
Contributor

Change https://go.dev/cl/458617 mentions this issue: cmd/compile: remove -d=typecheckinl flag

gopherbot

gopherbot commented on Dec 20, 2022

@gopherbot
Contributor

Change https://go.dev/cl/458622 mentions this issue: cmd/compile/internal/types: remove Type.Pkg

gopherbot

gopherbot commented on Dec 20, 2022

@gopherbot
Contributor

Change https://go.dev/cl/458619 mentions this issue: cmd/compile/internal/noder: stop creating TUNION types

gopherbot

gopherbot commented on Dec 20, 2022

@gopherbot
Contributor

Change https://go.dev/cl/458618 mentions this issue: cmd/compile: change some unreachable code paths into Fatalf

gopherbot

gopherbot commented on Dec 20, 2022

@gopherbot
Contributor

Change https://go.dev/cl/458575 mentions this issue: cmd/compile: revert package typecheck part of CL 422914

gopherbot

gopherbot commented on Dec 20, 2022

@gopherbot
Contributor

Change https://go.dev/cl/458620 mentions this issue: cmd/compile: remove GOEXPERIMENT=nounified frontend

gopherbot

gopherbot commented on Dec 20, 2022

@gopherbot
Contributor

Change https://go.dev/cl/458616 mentions this issue: cmd/compile/internal/pkginit: remove dependency on typecheck.Resolve

gopherbot

gopherbot commented on Dec 20, 2022

@gopherbot
Contributor

Change https://go.dev/cl/458621 mentions this issue: cmd/compile/internal/types: remove TTYPEPARAM and TUNION types

gopherbot

gopherbot commented on Dec 20, 2022

@gopherbot
Contributor

Change https://go.dev/cl/458615 mentions this issue: cmd: remove GOEXPERIMENT=nounified knob

14 remaining items

liggitt

liggitt commented on Feb 10, 2023

@liggitt
Contributor

FYI, we're seeing regressions on ARM builds in go1.20 that bisected to 833367e where the unified experiment was enabled by default (#58425 (comment))

Should the experiment remain until we're confident regressions in the unified build on go1.20 are resolved? If I understand correctly, go1.20 (released 10 days ago) is the first time the unified build was used by default, which means it has had very little time to be adopted

mdempsky

mdempsky commented on Feb 10, 2023

@mdempsky
ContributorAuthor

The nounified frontend used in the 1.18 and 1.19 releases was the first to support generics, but we've experienced a lot of subtle correctness issues with it, which became untenable to fix. E.g., for 1.18, we decided to stop backporting generics fixes, because every time we attempted to fix one issue, we introduced at least one new one.

The unified frontend has been in use internally within Google for months. The compiler team decided as a whole to commit to the unified frontend for 1.20 and forward. We talked about whether it made sense to keep the code around longer, but we have a lot of other work planned for 1.21 (e.g., overhauling how inlining works, for better PGO support), which was hindered by the technical burden of maintaining two frontends.

We did expect hiccups in the transition though, which is why GOEXPERIMENT=nounified is still available and supported in the 1.20 release branch. And admittedly #58339 is a bit embarrassing, but that will be fixed in 1.20.1. In 1.20, it can be worked around with -gcflags=all=-d=inlstaticinit=0.

Thanks for your patience while we address the reported regressions.

mdempsky

mdempsky commented on Feb 10, 2023

@mdempsky
ContributorAuthor

Closing this issue because the nounified frontend has been removed at tip. There's still more cleanup work to do, but that can happen without a tracking issue.

randall77

randall77 commented on Feb 10, 2023

@randall77
Contributor

@liggitt I don't think the "relocation truncated" error is particularly related to the unified frontend. That's just the change that happened to reorder functions such that the branch distance was exceeded.

When I compile your repro from #58425 successfully with 1.20, it generates a binary whose text size is 66MB, when the branch (call) distance on arm32 is +/- 32 MB. I'm kind of surprised it ever worked before - you probably just got lucky using the old frontend.

Not that there couldn't be reasons why the unified frontend exacerbates the situation, but once your program is >32MB of text it's just a matter of time before some benign change causes relocation errors. (Until #58425 is fixed, probably by adding trampolines or something.)

liggitt

liggitt commented on Feb 10, 2023

@liggitt
Contributor

When I compile your repro from #58425 successfully with 1.20, it generates a binary whose text size is 66MB, when the branch (call) distance on arm32 is +/- 32 MB. I'm kind of surprised it ever worked before - you probably just got lucky using the old frontend.

interesting... Kubernetes arm builds have been working since #17028, so 6+ years of luck was... pretty lucky

gopherbot

gopherbot commented on May 9, 2023

@gopherbot
Contributor

Change https://go.dev/cl/493995 mentions this issue: cmd/compile: update README.md for unified IR

locked and limited conversation to collaborators on May 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

FrozenDueToAgeNeedsFixThe path to resolution is known, but the work has not been done.compiler/runtimeIssues related to the Go compiler and/or runtime.

Type

No type

Projects

No projects

Relationships

None yet

    Development

    No branches or pull requests

      Participants

      @mdempsky@liggitt@randall77@gopherbot

      Issue actions

        cmd/compile: remove GOEXPERIMENT=nounified · Issue #57410 · golang/go