[PATCH] D49151: [SimplifyIndVar] Avoid generating truncate instructions with non-hoisted Load operand.

[Sebastian Pop via Phabricator via llvm-commits]

sebpop added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/IndVarSimplify.cpp:1597
+    if (widenWithVariantLoadUse(DU))
+      return nullptr;
+
----------------
az wrote:
> sebpop wrote:
> > sebpop wrote:
> > > ... and call it from here.
> > > 
> > >   if (analysisFails())
> > >     return nullptr;
> > >   codeGenWiden();
> > > 
> > this should be:
> > 
> >   if (analysisSucceeds()) {
> >      codeGenWiden();
> >      return nullptr;
> >   }
> Done what you have in mind which is completely separate analysis from code Gen but I still kept them in the same function with clearly marked analysis phase and codegen phase mainly because they share code that may need to be re-executed if separated. I Can separate them into two functions if you still think that this small enhancement to widening need an analysis function and a code gen function. 
Let's split the function into two smaller ones.


================
Comment at: llvm/lib/Transforms/Scalar/IndVarSimplify.cpp:1440
+				  User->getType() == WideType)) {
+        return false;
+      }
----------------
Let's break this down into smaller statements that can be read with ease:
here is my suggestion

  if (ExtKind == SignExtended) {
    for () {
      auto *User = ...;
      if (isa<SExtInst>(User) && User->getType() == WideType)
        continue;
      return false;
    }
  } else { // ExtKind == ZeroExtended
    for () {
      auto *User = ...;
      if (isa<ZExtInst>(User) && User->getType() == WideType)
        continue;
      return false;
    }
  }


https://reviews.llvm.org/D49151