[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