[PATCH] D78520: [InlineSpiller] simplify insertReload() NFC

Nick Desaulniers via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 20 17:22:49 PDT 2020


nickdesaulniers added a comment.

Thanks for the review!



================
Comment at: llvm/lib/CodeGen/InlineSpiller.cpp:934
 /// not relevant.
-static bool isFullUndefDef(const MachineInstr &Def) {
+static bool isRealSpill(const MachineInstr &Def) {
   if (!Def.isImplicitDef())
----------------
efriedma wrote:
> Not sure renaming this helper is really useful; the old name is the property it's actually computing, which makes more sense than naming it after what you plan to do with the result.
> 
> Not a big deal either way.
If we keep the old name, then we should not negate the return values as done in this diff. Instead, we'd negate the return value at the call site, which looks funny (just return what you want, no negation required).
```
bool IsRealSpill = !isFullUndefDef(*MI);
```
Thoughts? Also, doesn't matter to me.  This way just has fewer negations in it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78520/new/

https://reviews.llvm.org/D78520





More information about the llvm-commits mailing list