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

[Abderrazek Zaafrani via Phabricator via llvm-commits]

az marked 5 inline comments as done.
az added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/IndVarSimplify.cpp:1471
+        (isa<ZExtInst>(User) && (getExtendKind(NarrowUse) == ZeroExtended))) &&
+        (User->getType() == WideType)) {
+        LLVM_DEBUG(dbgs() << "INDVARS: eliminating " << *User
----------------
sebpop wrote:
> Can you please remove all the unneeded parentheses?
> Also I find it more clear if you first check for all cases that exit the loop with `WideningUseful = false; break;`
> 
> 
Removed one. I tried to remove another one that should work fine based on C precedence rule, but I got some compiler warning. So, it is slightly better but still parentheses heavy.


================
Comment at: llvm/lib/Transforms/Scalar/IndVarSimplify.cpp:1597
+    if (widenWithVariantLoadUse(DU))
+      return nullptr;
+
----------------
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. 


https://reviews.llvm.org/D49151