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

Eric Christopher echristo at gmail.com
Mon Mar 9 15:03:49 PDT 2015


On Mon, Mar 9, 2015 at 2:49 PM Mehdi AMINI <mehdi.amini at apple.com> wrote:

> Thanks for review, that was quick :)
>
>
>
Sure :)


> 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();
> ----------------
> echristo wrote:
> > Any reason not to leave it cached instead of passing it down?
> In short: cannot rebind a reference and I wanted to get rid of the pointer.
>
>
Ah, sure.


> It plays well with passes that initialize a separate transient object to
> do the processing, but not with passes that do their processing "in-place".
> Because passes object are not transient, they carry their state between
> runs, including a potential obsolete DL pointer.
>


> Not sure if I was clear?
>
>
Assuming in the case of "things that work on multiple modules". Otherwise,
something that just runs on a single module should be able to cache it with
no problem right? :)


> This was very case-by-case, and I may have get it wrong here and there (I
> mean wrong in the sense that a function argument could have been save by
> retrieving the DL differently, I don't think I broke correctness).
>
>
Right. Should be ok.


> ================
> 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
> ----------------
> echristo wrote:
> > Formatting. :)
> That was clang-format, it probably gets over 80 lines without the line
> break. I'll rephrase the comment to save what is needed to fit in the line.
>
>
Yep. Figured, it does that to me all the time, just calling it out.

-eric


> http://reviews.llvm.org/D8158
>
> EMAIL PREFERENCES
>   http://reviews.llvm.org/settings/panel/emailpreferences/
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150309/ca727371/attachment.html>


More information about the llvm-commits mailing list