[go1.20] Moving DCE to isolate filter naming #1337
Conversation
|
I think I'd like to take a step further and move all DCE-related logic into a separate |
9fc41fa to
e1c4f12
Compare
compiler/utils.go
Outdated
| // | ||
| // Note that calling CollectDCEDeps() inside another CollectDCEDeps() call is | ||
| // not allowed. | ||
| func (fc *funcContext) CollectDCEDeps(f func()) []string { | ||
| func (fc *funcContext) CollectDCEDeps(dce *dce.Info, f func()) { |
There was a problem hiding this comment.
Consider this an optional request, but I would move this method into the dce package as well.
There was a problem hiding this comment.
Moved to dce.Collector
compiler/internal/dce/info.go
Outdated
| } | ||
|
|
||
| // SetAsAlive marks the declaration as alive, meaning it will not be eliminated. | ||
| // This must be done after the SetName method is called. |
There was a problem hiding this comment.
Let's make this method panic if this invariant is violated.
There was a problem hiding this comment.
Instead of a panic, I made the explicitly marking as alive use an alive flag instead of clearing out the names. Now, if the names aren't set or if the alive flag is set true, the decl is alive. This allows the decl to be marked as alive before or after naming without effecting the naming.
compiler/internal/dce/selector.go
Outdated
There was a problem hiding this comment.
Since we are factoring this this logic out into its own package, it would be good to add at least some rudimentary tests
There was a problem hiding this comment.
Added some tests
6382c16 to
a802b89
Compare
a802b89 to
cacbfcf
Compare
|
@nevkontakte and @flimzy as I've been working on the generics DCE, I've pushed a few updates to this PR, but I think it has reached a stable point where this PR (hopefully) is a good foundation to build the generics DCE off of. However, I haven't fully figured out the generics DCE yet; I just didn't want to get this out and immediately change it all for generics 😆. Anyway, I think I've address the prior change requests and this PR should be ready for review again. It just occurred to me too. This PR is for the go1.20 branch but since a lot of the generics work is in master, should I be targeting master instead? |
|
Moving this over to master in #1339 |
The dead-code elimination (DCE) code has two filter strings and dependencies as a set of strings. The strings have special meanings such as both empty strings indicate always alive. To help facilitate future changes to DCE, I'm moving the DCE code to it's own package. Also I'm making some methods to set a decl's DCE filters and deps to ensure that the names are continued to be set the same as new changes are made.
These changes should have only minor functional changes, everything should continue to function as before. These changes are mostly a minor refactor to encapsulate DCE design decisions. The CI should continue to pass.
This is related to #1270 work since the future changes to DCE is to include removal of dead generic code instantiations, e.g. allow
List[int]to be alive andList[string]be dead separately.