[llvm-dev] Possible soundness issue with available_externally (split from "RFC: Add guard intrinsics")

[Chandler Carruth via llvm-dev]

On Fri, Feb 26, 2016 at 7:38 PM Hal Finkel <hfinkel at anl.gov> wrote:

> *From: *"Chandler Carruth" <chandlerc at google.com>
> *To: *"Hal Finkel" <hfinkel at anl.gov>
>
> *Cc: *"llvm-dev" <llvm-dev at lists.llvm.org>, "Philip Reames" <
> listmail at philipreames.com>, "Duncan P. N. Exon Smith" <
> dexonsmith at apple.com>, "Xinliang David Li" <xinliangli at gmail.com>,
> "Sanjoy Das" <sanjoy at playingwithpointers.com>
> *Sent: *Friday, February 26, 2016 9:33:55 PM
>
>
> *Subject: *Re: [llvm-dev] Possible soundness issue with
> available_externally (split from "RFC: Add guard intrinsics")
>
> On Fri, Feb 26, 2016 at 7:26 PM Hal Finkel <hfinkel at anl.gov> wrote:
>
>
>> > From: "Chandler Carruth" <chandlerc at google.com>
>> > To: "Hal Finkel" <hfinkel at anl.gov>, "Sanjoy Das" <
>> sanjoy at playingwithpointers.com>
>> > Cc: "llvm-dev" <llvm-dev at lists.llvm.org>, "Philip Reames" <
>> listmail at philipreames.com>, "Duncan P. N. Exon Smith"
>> > <dexonsmith at apple.com>, "Xinliang David Li" <xinliangli at gmail.com>
>> > Sent: Friday, February 26, 2016 9:01:48 PM
>> > Subject: Re: [llvm-dev] Possible soundness issue with
>> available_externally (split from "RFC: Add guard intrinsics")
>> >
>> >
>> > I think this will have a much higher cost than my proposal to
>> > constrain how we deduce function attributes (which still fixes
>> > Sanjoy's latest example).
>> >
>> > Specifically, I think this will force us to constrain far too many
>> > transformations for the sake of code size in functions that we won't
>> > inline. Even if we were never going to deduce function attributes
>> > for anything in the function (because its big and reads and writes
>> > everything), we'll still have to constrain our transformations just
>> > because we *might* later deduce a function attribute that triggers
>> > these kinds of bugs.
>> >
>> > Essentially, you're proposing to limit intraprocedural optimization
>> > to when we can successfully to interprocedural optimization
>> > ("privatization"), where I'm suggesting we limit interprocedural
>> > optimization to leave intraprocedural optimization unconstrained.
>> > Given the ratio of our optimizations (almost all are intra, very few
>> > are inter), I'm much more comfortable with the latter.
>>
>> This is a good point; we can certainly (easily) delay the privatization
>> decision until we modify any IPA-level function information (at which point
>> we can either reject the attribute change (when optimizing for code size),
>> or keep it locally (when optimizing for speed). Ideally, you'd want to
>> delay this even further (until you knew the attribute information was
>> used), but I'm not sure that's practical.
>>
>> Actually, what if instead of actually privatizing, we moved the function
>> into a different comdat section named after some hash of the function body?
>> That way, if all versions are actually optimized identically, we'll still
>> only end up with one copy in the final executable. If this is technically
>> possible, it seems like the best kind of solution.
>>
>
> This is how I want to do a revamped function merging anyways and it would
> fall out naturally of that.
>
> Excellent, so let's fix this at the same time we implement this function
> merging (so that we don't have performance regressions in an intermediate
> state). This will also allow us to have uniform logic across different
> optimization levels, which is obviously preferable.
>

I am *extremely* uncomfortable waiting to fix this until merging stuff is
in place and we add privatization heuristics to our IPO passes. Those might
be years away.

I think we should pretty immediately:

1) Make our IPO passes conservatively correc
2) Leave comments about how to add privatization, but explain the code size
cost incurred by it

I have no idea if anyone is even working on privatization (I'm not) or
function merging (no ETA at all, its really far down my queue). I think we
should decouple all these pieces. Once things are correct, folks can add a
very size-conservative privatization transformation to IPO routines if we
don't have merging, or a fairly aggressive one if we do have merging. And
if we add merging later we can re-tune the privatization.

-Chandler


>
> Thanks again,
> Hal
>
>
> --
> Hal Finkel
> Assistant Computational Scientist
> Leadership Computing Facility
> Argonne National Laboratory
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20160227/64a12a2a/attachment.html>