Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Maybe the popular RegisterRef can be moved to Register.h? Would that complicate this patch?
llvm/lib/Target/Hexagon/BitTracker.h | ||
---|---|---|
134 | are Reg and Pos const? | |
147 | Are Reg and Sub const? marking them so would help readability, and also ensure we don't accidentally yank the initializer in the ctor. | |
llvm/lib/Target/Hexagon/HexagonBlockRanges.h | ||
35 | weird... there was one of these earlier. Not sure if hoisting it in a common .h may make sense, but otherwise the comments are the same (const-ing Reg and Sub) | |
llvm/lib/Target/Hexagon/HexagonConstPropagation.cpp | ||
86–87 | seems like a popular design | |
llvm/lib/Target/Hexagon/HexagonExpandCondsets.cpp | ||
191–192 | 4th RegisterRef. Sub is probably unsigned? | |
llvm/lib/Target/Hexagon/HexagonGenPredicate.cpp | ||
51–52 | 5th RegisterRef :) |
I'd definitely prefer not cleaning up RegisterRef as part of this patch. The register/sub tuple pattern is present throughout the code and needs an independant clean-up. Otherwise we risk this not being a NFC.
Regarding const usage, this will delete the implicit copy constructor. Given the earlier statement that those classes need an independant clean-up, are you okay punting on this?
llvm/lib/Target/Hexagon/BitTracker.h | ||
---|---|---|
134 | See comments. | |
llvm/lib/Target/Hexagon/HexagonBlockRanges.h | ||
35 | Yeah this was very odd and requires clean-up | |
llvm/lib/Target/Hexagon/HexagonConstPropagation.cpp | ||
86–87 | See comments. |
Yes, but could you add a FIXME above each, with the same text - so we can fish them out easily afterwards.
are Reg and Pos const?