[PATCH] DataLayout is mandatory, update the API to reflect it with references.

Eric Christopher echristo at gmail.com
Mon Mar 9 10:31:30 PDT 2015


Couple of comments, in general looks fine and anything else can be covered in post-commit etc based on answers.

You pass down the DataLayout in a few new places rather than keep it cached or look it up, any reason? (This is also one of the inline comments as I found it in a particular place, but rather than keep asking it's also a top level comment).

Thanks for the work! That was very long :)

-eric


REPOSITORY
  rL LLVM

================
Comment at: lib/Analysis/Lint.cpp:178
@@ -177,3 +176,2 @@
   DT = &getAnalysis<DominatorTreeWrapperPass>().getDomTree();
-  DL = &F.getParent()->getDataLayout();
   TLI = &getAnalysis<TargetLibraryInfoWrapperPass>().getTLI();
----------------
Any reason not to leave it cached instead of passing it down?

================
Comment at: lib/Analysis/LoopAccessAnalysis.cpp:292
@@ -291,2 +291,3 @@
       if (hasComputableBounds(SE, StridesMap, Ptr) &&
-          // When we run after a failing dependency check we have to make sure we
+          // When we run after a failing dependency check we have to make sure
+          // we
----------------
Formatting. :)

http://reviews.llvm.org/D8158

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






More information about the llvm-commits mailing list