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

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

Comments

@mdempsky
Copy link
Member

@mdempsky mdempsky commented Jan 16, 2021

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

@gopherbot
Copy link

@gopherbot gopherbot commented Jan 16, 2021

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

Loading

@gopherbot
Copy link

@gopherbot gopherbot commented Jan 16, 2021

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

Loading

@gopherbot
Copy link

@gopherbot gopherbot commented Jan 16, 2021

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

Loading

gopherbot pushed a commit that referenced this issue Jan 17, 2021
… 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
…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
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

@gopherbot gopherbot commented Jan 18, 2021

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

Loading

@gopherbot
Copy link

@gopherbot gopherbot commented Jan 18, 2021

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

Loading

gopherbot pushed a commit that referenced this issue Jan 18, 2021
…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
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

@mdempsky mdempsky commented Jan 18, 2021

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

Loading

@mdempsky mdempsky closed this Jan 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants