[PATCH] D144089: [IndVarSimplify] Transform the trip count into a simpler form

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev requested changes to this revision.
mkazantsev added a comment.
This revision now requires changes to proceed.

1. I think it should be moved to existing utils and re-implemented using SCEVs rather than values. Most of prerequisite logic here looks duplicating the existing code.
2. Can you please provide a motivating test here? These tests with -O3 are too cryptic to understand what you are trying to achieve.



================
Comment at: llvm/lib/Transforms/Scalar/IndVarSimplify.cpp:2045
+  // Only optimize inner loops currently.
+  if (!L->getSubLoops().empty())
+    return false;
----------------
`L->isInnermost()`


================
Comment at: llvm/lib/Transforms/Scalar/IndVarSimplify.cpp:2049
+  // Only optimize simple loops.
+  if (L->getLoopLatch() != L->getHeader())
+    return false;
----------------
If so, the check above is redundant. Write a TODO to lift it? It practically happens very rarely.


================
Comment at: llvm/lib/Transforms/Scalar/IndVarSimplify.cpp:2086
+    TCValue = TC;
+  }
+
----------------
We already have this available in functions like `SimplifyIndvar::simplifyUsers`. It seems that you are replicating a lot of existing code. Did you try to integrate what you are doing into `SimplifyIndVars.cpp`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144089