[PATCH] D70009: [ARM][ReachingDefAnalysis] Use RDA for loloops

[Sam Parker via Phabricator via llvm-commits]

samparker marked 3 inline comments as done.
samparker added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp:149
+      const int CountDefId = RDA->getReachingDef(Start, CountReg);
+      const int LRDefId = RDA->getReachingDef(Start, ARM::LR);
+      if (CountDefId < LRDefId || CountDefId > StartId)
----------------
dmgreen wrote:
> This is getInstId(LRDef)?
good point.


================
Comment at: llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp:159
+      const int CountDefId = RDA->getReachingDef(LRDef, CountReg);
+      const int LRDefId = RDA->getReachingDef(Start, ARM::LR);
+      if (CountDefId > LRDefId || CountDefId < StartId)
----------------
dmgreen wrote:
> Is this one correct? (start as opposed to back)
I think I'll make some helpers for these, with more comments, because it's confusing me looking at it again!


================
Comment at: llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp:358
+  if (SetFlags) {
+    if (RDA->getReachingDef(&MBB->back(), ARM::CPSR) > RDA->getInstId(MI))
+      SetFlags = false;
----------------
dmgreen wrote:
> Does this change need the Defs = [CPSR] for t2LoopDec change? Are there some tests for these cases that didn't change?
It didn't need it in the end as this logic allows us to query for a CPSR def inbetween LoopDec (MI) and LoopEnd (back). Again, I'll create a helper which allows us to clearly whether we have a def of something inbetween A and B.


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

https://reviews.llvm.org/D70009