[PATCH] Modularizing LICM

Nema, Ashutosh Ashutosh.Nema at amd.com
Mon Feb 2 20:11:50 PST 2015


Thanks Bruno for reviewing this change.
Your suggestion looks fine, will work on it.

> Comment at: lib/Transforms/Scalar/LICM.cpp:995
> @@ +994,2 @@
> +}
> +
> ----------------
> Any specific reason why this was moved to the end of the file?

'inSubLoop' & ' pointerInvalidatedByLoop' was member function of 'LICM'
These are defined in class itself. Its required to move these out of LICM class
Because these has usage in exposed utility & dependent routines (i.e. SinkRegion).
For now these are kept in same file, but later I'm planning to move these to 
other utility file(i.e. LICMCore.cpp)

Regards,
Ashutosh

-----Original Message-----
From: Bruno Cardoso Lopes [mailto:bruno.cardoso at gmail.com] 
Sent: Tuesday, February 03, 2015 4:04 AM
To: Nema, Ashutosh; listmail at philipreames.com; bruno.cardoso at gmail.com
Cc: hfinkel at anl.gov; llvm-commits at cs.uiuc.edu
Subject: Re: [PATCH] Modularizing LICM

Hi,

The approach looks good. Aside from Hal's suggestions, only a few comments.


REPOSITORY
  rL LLVM

================
Comment at: include/llvm/Transforms/Utils/LoopUtils.h:92
@@ +91,3 @@
+/// PromoteAliasSet - Try to promote memory values to scalars by 
+sinking /// stores out of the loop and moving loads to before the loop.  
+We do this by /// looping over the stores in the loop, looking for 
+stores to Must pointers
----------------
Please don't repeat the function/method in the comments. Use "\brief" instead. Also fix this issue in other places of the patch too.

================
Comment at: lib/Transforms/Scalar/LICM.cpp:995
@@ +994,2 @@
+}
+
----------------
Any specific reason why this was moved to the end of the file?

http://reviews.llvm.org/D7291

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






More information about the llvm-commits mailing list