[PATCH] Modularizing LICM

[Nema, Ashutosh]

> Comment at: include/llvm/Transforms/Utils/LoopUtils.h:85
> @@ +84,3 @@
> +/// iteration. Takes DomTreeNode, AliasAnalysis, LoopInfo, 
> +DominatorTree, /// DataLayout, TargetLibraryInfo, Loop, AliasSet 
> +information for the input loop /// and loop safety information as argument. It returns changed status.
> ----------------
> Please improve the description of what needs to be in the AliasSetTracker. 
> Should it contain all memory accesses in the loop?

Sure Hal, will add more explanation here. 
Here alias set information for all the instructions in loop.

> +bool promoteAliasSet(AliasSet &, SmallVectorImpl<BasicBlock*> &,
> +                     SmallVectorImpl<Instruction*> &,
> This function seems poorly named given its description as, "Try to promote 
> memory values to scalars by sinking stores out of  the loop and moving loads 
> to before the loop." Maybe promoteLoopAccessesToScalars?

Looks OK, will update accordingly.

Regards,
Ashutosh

-----Original Message-----
From: hfinkel at anl.gov [mailto:hfinkel at anl.gov] 
Sent: Tuesday, February 17, 2015 7:44 PM
To: Nema, Ashutosh; listmail at philipreames.com; bruno.cardoso at gmail.com; hfinkel at anl.gov
Cc: llvm-commits at cs.uiuc.edu
Subject: Re: [PATCH] Modularizing LICM

REPOSITORY
  rL LLVM

================
Comment at: include/llvm/Transforms/Utils/LoopUtils.h:85
@@ +84,3 @@
+/// iteration. Takes DomTreeNode, AliasAnalysis, LoopInfo, 
+DominatorTree, /// DataLayout, TargetLibraryInfo, Loop, AliasSet 
+information for the input loop /// and loop safety information as argument. It returns changed status.
----------------
Please improve the description of what needs to be in the AliasSetTracker. Should it contain all memory accesses in the loop?


================
Comment at: include/llvm/Transforms/Utils/LoopUtils.h:110
@@ +109,3 @@
+/// argument. It returns changed status.
+bool promoteAliasSet(AliasSet &, SmallVectorImpl<BasicBlock*> &,
+                     SmallVectorImpl<Instruction*> &,
----------------
This function seems poorly named given its description as, "Try to promote memory values to scalars by sinking stores out of  the loop and moving loads to before the loop." Maybe promoteLoopAccessesToScalars?

http://reviews.llvm.org/D7291

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/