[PATCH] D91358: [ARM][RegAlloc] Add t2LoopEndDec

[Mircea Trofin via Phabricator via llvm-commits]

mtrofin added inline comments.


================
Comment at: llvm/lib/CodeGen/CalcSpillWeights.cpp:227
+    // not spillable.
+    if (MI->isTerminator() && MI->definesRegister(LI.reg()) &&
+        TII.isUnspillableTerminator(MI)) {
----------------
SjoerdMeijer wrote:
> dmgreen wrote:
> > SjoerdMeijer wrote:
> > > Nit: guess you don't really need to check isTerminator as that will be checked in isUnspillableTerminator?
> > I was trying to make it explicit, that it needed to be a terminator that defined the value and this shouldn't be used for anything else.
> > 
> > I can change that if you think it's better to remove it. It is currently just trying to be conservative.
> Not a big deal, whatever you prefer.
from a readability perspective, isUnspillableTerminator suggests "isTerminator && isUnspillable".

How about having isUnspillableTerminator not virtual and doing this:

return isTerminator() && isUnspillableTerminatorImpl() 

where isUnspillableTerminatorImpl is protected and virtual?


================
Comment at: llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp:1375
+  MIB.addImm(ARMCC::AL);
+  MIB.addReg(0);
+  MIB.addReg(ARM::CPSR);
----------------
Nit: addReg(MCRegister::NoRegister) is probably more readable.


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

https://reviews.llvm.org/D91358