Reland "MTM: fix issues after cursory reading" #101191
Conversation
Change: Using ArrayRef{} constructor syntax causes a build failure on
gcc-7. Hence, revert instances of this change.
|
(passing by) What issues does it fix? I only see questionable style changes. |
See #100404 for the original discussion: I initially marked it as a style change, but a reviewer requested dropping the NFC tag. |
| @@ -133,7 +127,7 @@ MachineTraceMetrics::getResources(const MachineBasicBlock *MBB) { | |||
|
|
|||
| // Scale the resource cycles so they are comparable. | |||
| unsigned PROffset = MBB->getNumber() * PRKinds; | |||
| for (unsigned K = 0; K != PRKinds; ++K) | |||
| for (unsigned K = 0; K < PRKinds; ++K) | |||
There was a problem hiding this comment.
The coding standard doesn't specify which style of control condition is preferred, so there is no reason to change this line.
| @@ -546,7 +540,7 @@ MachineTraceMetrics::Ensemble::invalidate(const MachineBasicBlock *BadMBB) { | |||
| if (BadTBI.hasValidHeight()) { | |||
| BadTBI.invalidateHeight(); | |||
| WorkList.push_back(BadMBB); | |||
| do { | |||
| while (!WorkList.empty()) { | |||
There was a problem hiding this comment.
No reason to change this either, especially when it is clear that the container is non-empty on entry.
| @@ -605,7 +599,7 @@ void MachineTraceMetrics::Ensemble::verify() const { | |||
| #ifndef NDEBUG | |||
| assert(BlockInfo.size() == MTM.MF->getNumBlockIDs() && | |||
| "Outdated BlockInfo size"); | |||
| for (unsigned Num = 0, e = BlockInfo.size(); Num != e; ++Num) { | |||
| for (unsigned Num = 0; Num < BlockInfo.size(); ++Num) { | |||
There was a problem hiding this comment.
The left form is preferred.
| std::tie(I, New) = Heights.insert(std::make_pair(Dep.DefMI, UseHeight)); | ||
| if (New) | ||
| const auto &[I, Inserted] = Heights.insert({Dep.DefMI, UseHeight}); | ||
| if (Inserted) |
There was a problem hiding this comment.
The coding standard says nothing about structured bindings. But it says something about auto -- that using it is discouraged, unless it clearly improves readability.
|
Not pursuing this. |
Change: Using ArrayRef{} constructor syntax causes a build failure on gcc-7. Hence, revert instances of this change.