[RegAllocFast] NFC cleanups #74860
[RegAllocFast] NFC cleanups #74860
Conversation
nickdesaulniers
commented
Dec 8, 2023
nickdesaulniers
commented
- use more range for
- avoid capturing lambda
- prefer Register type to unsigned
- remove braces around single statement if
- use more range for - avoid capturing lambda - prefer Register type to unsigned - remove braces around single statement if
| // Scan for special cases; Apply pre-assigned register defs to state. | ||
| bool HasPhysRegUse = false; | ||
| bool HasRegMask = false; | ||
| bool HasVRegDef = false; | ||
| bool HasDef = false; | ||
| bool HasEarlyClobber = false; | ||
| bool NeedToAssignLiveThroughs = false; | ||
| for (unsigned I = 0; I < MI.getNumOperands(); ++I) { | ||
| MachineOperand &MO = MI.getOperand(I); | ||
| for (MachineOperand &MO : MI.operands()) { |
There was a problem hiding this comment.
This feels like it was intentional, especially given it is not even storing MI.getNumOperands() in a variable. And in fact git blame points to this: https://reviews.llvm.org/D124834
There was a problem hiding this comment.
It was done to pass I to TiedOpIsUndef, not because MI's number of operands change in the loop. I've eliminated the need to pass I, since it can be rematerialized from the MachineOperand itself.
|
Most of the changes look good to me. I suspect there is ways where |
|
CC @LuoYuanke I assume you changed those for-loops on purpose in https://reviews.llvm.org/D124834 ? Do you still remember why? As admittedly I cannot immediately see the reason for it, especially with that reverse-loop where added operands would not even get recognized... |
