[PATCH] D103951: [NFC][Scheduler] Refactor tryCandidate to return boolean
Qiu Chaofan via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Jun 28 02:35:15 PDT 2021
qiucf added inline comments.
================
Comment at: llvm/lib/CodeGen/MachineScheduler.cpp:3191
// Bias PhysReg Defs and copies to their uses and defined respectively.
if (tryGreater(biasPhysReg(TryCand.SU, TryCand.AtTop),
biasPhysReg(Cand.SU, Cand.AtTop), TryCand, Cand, PhysReg))
----------------
foad wrote:
> Surely all of these "try" functions should consistently only return true when they set Reason to something useful? So all these clauses should just be:
> ```
> if (trySomething(...))
> return true;
> ```
I think current implementation is for saving compilation time (`tryCandidate` should be very hot):
- If `TryCand` is better than `Cand`, return true;
- If `Cand` is better than `TryCand`, it also returns true, since there's no need to execute comparisons below;
- Only if we don't know which is better, go next check.
```
/// Return true if this heuristic determines order.
bool tryLess(int, int, SchedCandidate, SchedCandidate, CandReason);
```
(//return true if order determined// vs. //return true if TryCand is better//)
So the proposed change looks better, but seems it'll increase compilation time.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D103951/new/
https://reviews.llvm.org/D103951
More information about the llvm-commits
mailing list