[PATCH] D21720: Unroll for uncountable loops

[Evgeny Stupachenko via Phabricator via llvm-commits]

evstupac added inline comments.


================
Comment at: lib/Transforms/Scalar/LoopUnrollPass.cpp:430-434
+      if ((TripCount & 1) == 0) {
+        if (IL && IL->getOpcode() == Instruction::Xor &&
+            IL->getOperand(0) == PHI && L->isLoopInvariant(IL->getOperand(1))) {
+          Constant *CP = dyn_cast<Constant>(VP);
+          if (Iteration & 1)
----------------
mzolotukhin wrote:
> This look very hacky. What is the motivation of this change? How often do we see such loops?
> 
> If we do want to handle such cases (which I'm not convinced now), then we should do it in a general way. That is, the logic should be in instruction visitors, and we should automagically deduce that these instructions are free. There are more cases than just xor - we can multiply by -1, and we should be able to handle in a similar way. In the current form the code is not easily extensible to handle new cases.
>What is the motivation of this change? How often do we see such loops?
Not each test looks like this, but there are couple where we switch states:
  state = st[s^=1];
which becomes invariant with some other calculations after unroll.
>If we do want to handle such cases (which I'm not convinced now), then we should do it in a general way. That is, the logic should be in instruction visitors, and we should automagically deduce that these instructions are free. 
The same is valid for the code above where complete unroll simplify phi. Why phi is simplified here?
I just did the same for XOR.
And yes we can multiply by -1, do i&1, i/2,... but we need a start point which depends on unroll factor and iteration.
The other solution is to pass, unroll factor in addition to iteration number and move simplification there.
Do you think this is better?



Repository:
  rL LLVM

https://reviews.llvm.org/D21720