[llvm-dev] Possible soundness issue with available_externally (split from "RFC: Add guard intrinsics")
Philip Reames via llvm-dev
llvm-dev at lists.llvm.org
Mon Feb 29 14:58:02 PST 2016
On 02/26/2016 08:04 PM, Chandler Carruth wrote:
> On Fri, Feb 26, 2016 at 7:59 PM Hal Finkel <hfinkel at anl.gov
> <mailto:hfinkel at anl.gov>> wrote:
>
> *From: *"Chandler Carruth" <chandlerc at google.com
> <mailto:chandlerc at google.com>>
> *To: *"Hal Finkel" <hfinkel at anl.gov <mailto:hfinkel at anl.gov>>
> *Cc: *"llvm-dev" <llvm-dev at lists.llvm.org
> <mailto:llvm-dev at lists.llvm.org>>, "Philip Reames"
> <listmail at philipreames.com
> <mailto:listmail at philipreames.com>>, "Duncan P. N. Exon Smith"
> <dexonsmith at apple.com <mailto:dexonsmith at apple.com>>,
> "Xinliang David Li" <xinliangli at gmail.com
> <mailto:xinliangli at gmail.com>>, "Sanjoy Das"
> <sanjoy at playingwithpointers.com
> <mailto:sanjoy at playingwithpointers.com>>
>
> *Sent: *Friday, February 26, 2016 9:41:23 PM
>
>
> *Subject: *Re: [llvm-dev] Possible soundness issue with
> available_externally (split from "RFC: Add guard intrinsics")
>
> On Fri, Feb 26, 2016 at 7:38 PM Hal Finkel <hfinkel at anl.gov
> <mailto:hfinkel at anl.gov>> wrote:
>
> *From: *"Chandler Carruth" <chandlerc at google.com
> <mailto:chandlerc at google.com>>
> *To: *"Hal Finkel" <hfinkel at anl.gov
> <mailto:hfinkel at anl.gov>>
>
> *Cc: *"llvm-dev" <llvm-dev at lists.llvm.org
> <mailto:llvm-dev at lists.llvm.org>>, "Philip Reames"
> <listmail at philipreames.com
> <mailto:listmail at philipreames.com>>, "Duncan P. N.
> Exon Smith" <dexonsmith at apple.com
> <mailto:dexonsmith at apple.com>>, "Xinliang David Li"
> <xinliangli at gmail.com <mailto:xinliangli at gmail.com>>,
> "Sanjoy Das" <sanjoy at playingwithpointers.com
> <mailto: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 <mailto:hfinkel at anl.gov>> wrote:
>
>
> > From: "Chandler Carruth" <chandlerc at google.com
> <mailto:chandlerc at google.com>>
> > To: "Hal Finkel" <hfinkel at anl.gov
> <mailto:hfinkel at anl.gov>>, "Sanjoy Das"
> <sanjoy at playingwithpointers.com
> <mailto:sanjoy at playingwithpointers.com>>
> > Cc: "llvm-dev" <llvm-dev at lists.llvm.org
> <mailto:llvm-dev at lists.llvm.org>>, "Philip Reames"
> <listmail at philipreames.com
> <mailto:listmail at philipreames.com>>, "Duncan P. N.
> Exon Smith"
> > <dexonsmith at apple.com
> <mailto:dexonsmith at apple.com>>, "Xinliang David
> Li" <xinliangli at gmail.com
> <mailto: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'm *extremely* uncomfortable fixing this at all unless it can be
> done without causing performance regressions. The underlying basic
> use case (linking together code compiled with different
> optimization levels), is certainly something I'd like to work
> properly, but is definitely a far lower priority than optimized
> code quality and size.
>
>
> Ok, I prioritize this differently: it is absolutely critical. Every
> binary I have includes files that fit this description. Every single
> binary.
Chandler, I'm more in agreement with Hal here. I see your point, but
given gcc appears to be getting this equally wrong, and we've never seen
this case in practice, I don't see fixing this as a critical issue. I'm
also strongly in agreement with Hal that a fix which reverts performance
for the normal case is unacceptable unless all options have been
explored and there really are no other choices.
>
> I think that we will discover subtle miscompiles every time we try to
> make function attributes more powerful until this is fixed. I think
> this is absolutely critical to fix.
Not sure this is actual true. In particular, there have been several
changes to our IPO passes over the last year and I don't remember
hearing any screaming. :)
Philip
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20160229/ce4a3fa8/attachment.html>
More information about the llvm-dev
mailing list