Organize SILOptimizer/Utils headers. Remove Local.h. #27417
Conversation
atrick
requested review from
eeckstein,
aschwaighofer,
gottesmm and
nate-chandler
|
@swift-ci test |
|
|
||
| /// Return true if there are any users of V outside the specified block. | ||
| inline bool isUsedOutsideOfBlock(SILValue V) { | ||
| auto *BB = V->getParentBlock(); |
There was a problem hiding this comment.
As an example of where the style is no beuano b/c of the age of the code.
There was a problem hiding this comment.
I suppose I could make a pass over the code and convert variable names to lower camel. Are there any other specific conventions that you think need to be fixed? I don't want to rewrite anything in this PR.
There was a problem hiding this comment.
I am fine with that and would like a git-clang-format as well. I would do it as a separate commit for cleanliness purposes. (I don't think it needs to be a separate PR, but you do you?).
If you do do this, please make it a separate commit from the move. Otherwise it's a lot harder to track back the history of the file. |
|
@jrose-apple (noticing the -1). git's diff is good enough today to do renames of cases/ignore whitespace changes when doing a blame. That being said, I also think that doing this in a separate commit is ok from my perspective (and slightly cleaner). |
|
@jrose-apple I strongly agree--moving code around should just move the code without other lexical changes. I'll try to add one extra commit for camelCase, and a second extra commit for clang-format. I took @gottesmm's suggestion to be purely from a social engineering standpoint: when you take to cleanup code, just clean it up all the way. That said, I have an important bug fix blocked on this so might end up splitting PRs if it takes too long. |
|
@swift-ci test |
|
Build failed |
|
Build failed |
|
@swift-ci test linux platform |
|
SGTM |
|
@swift-ci smoke test |
The XXOptUtils.h convention is already established and parallels the SIL/XXUtils convention. New: - InstOptUtils.h - CFGOptUtils.h - BasicBlockOptUtils.h - ValueLifetime.h Removed: - Local.h - Two conflicting CFG.h files This reorganization is helpful before I introduce more utilities for block cloning similar to SinkAddressProjections. Move the control flow utilies out of Local.h, which was an unreadable, unprincipled mess. Rename it to InstOptUtils.h, and confine it to small APIs for working with individual instructions. These are the optimizer's additions to /SIL/InstUtils.h. Rename CFG.h to CFGOptUtils.h and remove the one in /Analysis. Now there is only SIL/CFG.h, resolving the naming conflict within the swift project (this has always been a problem for source tools). Limit this header to low-level APIs for working with branches and CFG edges. Add BasicBlockOptUtils.h for block level transforms (it makes me sad that I can't use BBOptUtils.h, but SIL already has BasicBlockUtils.h). These are larger APIs for cloning or removing whole blocks.
Requested by gottesmm during review. Update the variable naming conventions to lower-camel. Run clang-format. I'm sure I missed some local variables somewhere--this was a best effort.
|
@swift-ci smoke test and merge |
1 similar comment
|
@swift-ci smoke test and merge |
The XXOptUtils.h convention is already established and parallels
the SIL/XXUtils convention.
New:
Removed:
This reorganization is helpful before I introduce more
utilities for block cloning similar to SinkAddressProjections.
Move the control flow utilies out of Local.h, which was an
unreadable, unprincipled mess. Rename it to InstOptUtils.h, and
confine it to small APIs for working with individual instructions.
These are the optimizer's additions to /SIL/InstUtils.h.
Rename CFG.h to CFGOptUtils.h and remove the one in /Analysis. Now
there is only SIL/CFG.h, resolving the naming conflict within the
swift project (this has always been a problem for source tools). Limit
this header to low-level APIs for working with branches and CFG edges.
Add BasicBlockOptUtils.h for block level transforms (it makes me sad
that I can't use BBOptUtils.h, but SIL already has
BasicBlockUtils.h). These are larger APIs for cloning or removing
whole blocks.