[go] cmd/compile: make dead auto elim ignore variable size
2 views
Skip to first unread message
Junyang Shao (Gerrit)
12:58 AM (18 hours ago) 12:58 AM
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Junyang Shao has uploaded the change for review![]( "Open in Gerrit")
Commit message
cmd/compile: make dead auto elim ignore variable size
This CL is to demonstrate that we don't actually need to trace the
variable size for struct indexing, as the size and type of
OpMove/OpStore/OpZero/OpLoad are more reliable than
LocalAddr/Addr/OffPtr, because ssa->prog relies on them too.
It looks like size=-1 and isStructNoPtr could be merged to another field
`valid`.
WIP and can be collapsed to CL 706555.
Change-Id: I701e98c35dc8733382de868fb5fe3de207d01e20
Change diff
diff --git a/src/cmd/compile/internal/ssa/deadstore.go b/src/cmd/compile/internal/ssa/deadstore.go
index 184351a..25a99ac 100644
--- a/src/cmd/compile/internal/ssa/deadstore.go
+++ b/src/cmd/compile/internal/ssa/deadstore.go
@@ -351,11 +351,13 @@
type nameIdx struct {
name *ir.Name
off int64
+ // size = -1 means invalid size, or unknown size if it's in [addr].
size int64
// isStructNoPtr is set [name] is of a struct type with no pointer fields,
// only when it's set [off] and [size] are traced correctly.
isStructNoPtr bool
}
+ // The [size] field of the value in addr will all be -1.
addr := make(map[*Value]nameIdx) // values that the address of the auto reaches
elim := make(map[*Value]nameIdx) // values that could be eliminated if the auto is
used := map[nameIdx]struct{}{} // used autos that must be kept
@@ -375,15 +377,7 @@
panic("addr pointer elem size mismatches name size")
}
if _, ok := addr[v]; !ok {
- canAnalyzeOffSize := nameType.IsStruct() && types.PtrDataSize(nameType) == 0 &&
- // Looks like for generics, some stub struct types could appear to be sized 0,
- // and when interacting with unsafe.Pointer magics, [nameType.Size()] can no
- // longer reflect the actual underlying memory size correctly.
- // So we have to check this as well. Without this check dead auto elim will
- // panic when compiling [TypeAssert] function in reflect/value.go with
- // GOEXPERIMENT=jsonv2.
- nameType.Size() > 0
- addr[v] = nameIdx{n, 0, nameType.Size(), canAnalyzeOffSize}
+ addr[v] = nameIdx{n, 0, -1, nameType.IsStruct() && types.PtrDataSize(nameType) == 0}
changed = true
}
return
@@ -399,14 +393,7 @@
if a.Op == OpAddr || a.Op == OpLocalAddr {
if n, ok := addr[a]; ok && n.isStructNoPtr {
if _, ok2 := addr[v]; !ok2 {
- if offset < 0 {
- panic("struct index underflow")
- }
- if offset >= n.size {
- panic("struct index overflow")
- }
n.off = offset
- n.size -= offset
addr[v] = n
changed = true
}
@@ -422,7 +409,7 @@
return
}
if _, ok := elim[v]; !ok {
- elim[v] = nameIdx{n, 0, n.Type().Size(), false}
+ elim[v] = nameIdx{n, 0, -1, false}
changed = true
}
return
@@ -437,7 +424,7 @@
if !ok || n.Class != ir.PAUTO {
return
}
- no := nameIdx{n, 0, n.Type().Size(), false}
+ no := nameIdx{n, 0, -1, false}
if _, ok := used[no]; !ok {
used[no] = struct{}{}
changed = true
@@ -451,9 +438,6 @@
if v.Op == OpStore {
writeSize = v.Aux.(*types.Type).Size()
}
- if n.isStructNoPtr && writeSize > n.size {
- panic("write size out bound")
- }
n.size = writeSize
elim[v] = n
changed = true
@@ -484,9 +468,6 @@
if n.isStructNoPtr {
// TODO: we should handle more than Load.
if v.Op == OpLoad {
- if v.Type.Size() > n.size {
- panic("read size out bound")
- }
n.size = v.Type.Size()
} else {
// Not an op we currently can reason, or the load out of bound of the variable
@@ -576,7 +557,7 @@
usedByName := make(map[*ir.Name]accessIntervals)
for n := range used {
allNameUsed.Add(n.name)
- if !n.isStructNoPtr {
+ if !n.isStructNoPtr || n.size == -1 {
nameNoOffSize.Add(n.name)
}
if _, ok := nameNoOffSize[n.name]; ok {
@@ -602,7 +583,7 @@
if _, ok := nameNoOffSize[n.name]; ok {
continue
}
- if !n.isStructNoPtr {
+ if !n.isStructNoPtr || n.size == -1 {
// This elim candidate does not have a valid offset and size or we have not yet well
// reasoned about then, so any nameIdx uses that touches this name should prevent
// this elim candidate from being removed.
Change information
Files:
- M src/cmd/compile/internal/ssa/deadstore.go
Change size: S
Delta: 1 file changed, 7 insertions(+), 26 deletions(-)
Related details
Attention set is empty
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Junyang Shao (Gerrit)
1:03 PM (5 hours ago) 1:03 PM
to goph...@pubsubhelper.golang.org, David Chase, Keith Randall, golang-co...@googlegroups.com
Attention needed from David Chase and Keith Randall
Junyang Shao voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Related details
Attention is currently required from:
- David Chase
- Keith Randall
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages