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.

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: change ir.NameOffsetExpr to use *obj.LSym instead of *Name #43737

Closed
mdempsky opened this issue Jan 16, 2021 · 6 comments
Closed

cmd/compile: change ir.NameOffsetExpr to use *obj.LSym instead of *Name #43737

mdempsky opened this issue Jan 16, 2021 · 6 comments
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Unplanned

Comments

@mdempsky
Copy link
Member

It looks like NameOffsetExpr is always used with global variables (i.e., ONAME/PEXTERN). In that case, we can replace the Name_ *Name field with Linksym *obj.LSym. We might even rename it to LinksymExpr.

This would then allow reflectdata.TypePtr and reflectdata.ITabAddr to create NameOffsetExprs instead of Names, which moves us towards avoiding use of types.Sym for compiler-generated symbols.

/cc @cuonglm
@mdempsky mdempsky added help wanted NeedsFix The path to resolution is known, but the work has not been done. labels Jan 16, 2021
@mdempsky mdempsky added this to the Unplanned milestone Jan 16, 2021
@gopherbot
Copy link

Change https://golang.org/cl/284118 mentions this issue: [dev.regabi] cmd/compile: stop analyze NameOffsetExpr.Name_ in escape analysis

@gopherbot
Copy link

Change https://golang.org/cl/284120 mentions this issue: [dev.regabi] cmd/compile: rename NameOffsetExpr to LinksymOffsetExpr

@gopherbot
Copy link

Change https://golang.org/cl/284119 mentions this issue: [dev.regabi] cmd/compile: change ir.NameOffsetExpr to use *obj.LSym instead of *Name

gopherbot pushed a commit that referenced this issue Jan 17, 2021
@cuonglm
[dev.regabi] cmd/compile: stop analyze NameOffsetExpr.Name_ in escape…
88956fc 
… analysis

It is always used with global variables, so we can skip analyze it, the
same as what we are doing for ONAME/PEXTERN nodes.

While at it, add a Fatalf check to ensure NewNameOffsetExpr is only
called for global variables.

For #43737

Change-Id: Iac444ed8d583baba5042bea096531301843b1e8f
Reviewed-on: https://go-review.googlesource.com/c/go/+/284118
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
gopherbot pushed a commit that referenced this issue Jan 17, 2021
@cuonglm
[dev.regabi] cmd/compile: change ir.NameOffsetExpr to use *obj.LSym i…
82b9cae 
…nstead of *Name

Because NameOffsetExpr is always used with global variables, and SSA
backend only needs (*Name).Linksym() to generate value for them.

Passes toolstash -cmp.

Updates #43737

Change-Id: I17209e21383edb766070c0accd1fa4660659caef
Reviewed-on: https://go-review.googlesource.com/c/go/+/284119
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
gopherbot pushed a commit that referenced this issue Jan 17, 2021
@cuonglm
[dev.regabi] cmd/compile: rename NameOffsetExpr to LinksymOffsetExpr
59ff93f 
Updates #43737

[git-generate]

cd src/cmd/compile/internal/ir

rf '
  mv NameOffsetExpr LinksymOffsetExpr
  mv ONAMEOFFSET OLINKSYMOFFSET
'

go generate

Change-Id: I8c6b8aa576e88278c0320d16bb2e8e424a15b907
Reviewed-on: https://go-review.googlesource.com/c/go/+/284120
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
@gopherbot
Copy link

Change https://golang.org/cl/284121 mentions this issue: [dev.regabi] cmd/compile: use *obj.LSym instead of *ir.Name for staticdata functions

@gopherbot
Copy link

Change https://golang.org/cl/284122 mentions this issue: [dev.regabi] cmd/compile: use LinksymOffsetExpr in TypePtr/ItabAddr

gopherbot pushed a commit that referenced this issue Jan 18, 2021
@cuonglm
[dev.regabi] cmd/compile: use *obj.LSym instead of *ir.Name for stati…
0ffa1ea 
…cdata functions

Those functions only use (*ir.Name).Linksym(), so just change them to
get an *obj.LSym directly. This helps get rid of un-necessary
validations that their callers have already done.

Passes toolstash -cmp.

For #43737.

Change-Id: Ifd6c2525e472f8e790940bc167665f9d74dd1bc5
Reviewed-on: https://go-review.googlesource.com/c/go/+/284121
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
gopherbot pushed a commit that referenced this issue Jan 18, 2021
@cuonglm
[dev.regabi] cmd/compile: use LinksymOffsetExpr in TypePtr/ItabAddr
4c835f9 
Passes toolstash -cmp.

Fixes #43737

Change-Id: I2d5228c0213b5f8742e3cea6fac9bc985b19d78c
Reviewed-on: https://go-review.googlesource.com/c/go/+/284122
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
@mdempsky
Copy link
Member Author

This is done on dev.regabi now. Thanks @cuonglm!

@golang golang locked and limited conversation to collaborators Jan 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
2 participants
@mdempsky @gopherbot