[PATCH] D104659: [ARM] Limit v6m unrolling with multiple live outs

[Sjoerd Meijer via Phabricator via llvm-commits]

SjoerdMeijer added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMTargetTransformInfo.cpp:2209
+      });
+      ExitingValues = ExitingValues < LiveOuts ? LiveOuts : ExitingValues;
+    }
----------------
dmgreen wrote:
> SjoerdMeijer wrote:
> > I was wondering if this is right. I.e., we are not e.g. accumulating the exit values, or taking a maximum, or something like that, if I am not mistaken. 
> I would expect that most loops have a single exit, but the code above does allow (at most) 2 exit.
> 
> From those I took the maximum of the number of liveouts as a rough heuristic, as I would expect the liveouts to be the same in many cases for multi-exit loops, and it probably doesn't want to count them twice.
Ah yes, I now see:

  if (ExitingBlocks.size() > 2)
    return;

above and also that the maximum is taken, thanks.


================
Comment at: llvm/test/Transforms/LoopUnroll/ARM/v6munroll.ll:52
+  %cmp.not = icmp eq i32 %dec, 0
+  br i1 %cmp.not, label %for.cond.cleanup.loopexit, label %for.body
+
----------------
One nit/question about the test case then, would it be useful to have a test and loop with 2 exit blocks?


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

https://reviews.llvm.org/D104659