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

Eli Friedman via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 20 17:55:03 PDT 2020


efriedma added inline comments.


================
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())
----------------
nickdesaulniers wrote:
> 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.
If you might want to use this helper elsewhere, then making the helper positive makes more sense.  But thinking about it a bit more, probably fine to leave your patch as-is, and we can reconsider if we ever find some other place that wants to use 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