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

[Sjoerd Meijer via Phabricator via llvm-commits]

SjoerdMeijer added subscribers: bjope, mtrofin, arsenm.
SjoerdMeijer added a comment.

Adding @mtrofin, @bjope, @arsenm for the unspillable terminator change.



================
Comment at: llvm/lib/CodeGen/CalcSpillWeights.cpp:227
+    // not spillable.
+    if (MI->isTerminator() && MI->definesRegister(LI.reg()) &&
+        TII.isUnspillableTerminator(MI)) {
----------------
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.


================
Comment at: llvm/lib/Target/ARM/ARMInstrThumb2.td:5451
 
+def t2LoopEndDec :
+  t2PseudoInst<(outs GPRlr:$Rm), (ins GPRlr:$elts, brtarget:$target),
----------------
dmgreen wrote:
> SjoerdMeijer wrote:
> > bikeshedding names: how about `t2LoopDecEnd` to more keep the semantics this is a decrement + end, in that order?
> Yeah, It does both together. I have no strong opinion on the order in the name. I guess I chose t2LoopEndDec as I felt that the major thing it did was behave as a loop end (like an LE instruction). The Dec somewhat of a side effect of that.
> 
> Can change it if you think it's better the other way. Let me know.
No strong opinion on this, I was bikeshedding anyway.


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

https://reviews.llvm.org/D91358