[PATCH] D105802: [LoopFlatten] Fix missed LoopFlatten opportunity

[Sjoerd Meijer via Phabricator via llvm-commits]

SjoerdMeijer added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/LoopFlatten.cpp:175
+    Value *LatchValue = InductionPHI->getIncomingValueForBlock(Latch);
+    if (match(LatchValue,
+              m_c_Add(m_Specific(InductionPHI), m_ConstantInt<1>()))) {
----------------
fhahn wrote:
> Those kinds of patterns are very fragile in general. Would it be possible to use SCEV instead, which makes it easy to analyse the induction variables & trip counts in a more robust way?
You're absolutely right. In fact, there's a quite some pattern matching going on for things like getLoopTest, getLoopIncrement, etc., that you'd probably expect to be present in a loop util helper function or something like that, but they aren't. The pattern matching makes some things easier though (as we are looking for some specific patterns). I feel that moving some things to helpers and using SCEV is major surgery, and if we promise not to add more pattern matching after this, I was hoping to postpone this surgery. I.e., we are on a little mission to get LoopFlatten enabled by default (we have this enabled for years now downstream), and before we do that we wanted to fix 2 minor things:
- this minor extension for a case which you expect to trigger,
- and we need to strengthen an overflow check.

After this, I think a nice follow up project is indeed to see if we can move code to helpers and use SCEV.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105802