[llvm-commits] [PATCH] Invariants (and Assume Aligned) - LLVM
Hal Finkel
hfinkel at anl.gov
Sun Dec 9 22:00:59 PST 2012
----- Original Message -----
> From: "Andrew Trick" <atrick at apple.com>
> To: "Dan Gohman" <dan433584 at gmail.com>
> Cc: reviews+D150+public+94bd4758cb777256 at llvm-reviews.chandlerc.com, llvm-commits at cs.uiuc.edu
> Sent: Friday, December 7, 2012 8:57:22 PM
> Subject: Re: [llvm-commits] [PATCH] Invariants (and Assume Aligned) - LLVM
>
> Hi Dan. Nice explanation. I have some questions below, but mostly
> evangelism.
>
> On Dec 5, 2012, at 5:03 PM, Dan Gohman <dan433584 at gmail.com> wrote:
> > This patch assumes that asserted conditions are globally true. This
> > significantly restricts their usefulness. Everything's easy if we
> > just
> > think about asserting properties for an entire function:
> >
> > void foo(void *p) {
> > __builtin_assume(p is aligned or whatever);
> > use(p);
> > }
> >
> > However, as soon as a function like this can get inlined into a
> > perfectly reasonable callsite like this:
> >
> > if (p is aligned or whatever)
> > foo(p);
> > else
> > do_something_slow(p);
> >
> > it doesn't work anymore, because it would apply to all uses of p in
> > the caller, including uses where p isn't aligned.
> >
> > It seems that the intuitive semantics would be that an assertion
> > should hold for all the uses dominated by an assertion statement.
> > Requiring dominator trees for anything that wants to use assertion
> > information would unfortunately keep it out of instcombine, where
> > it
> > would be very useful, so this is a significant problem. The
> > tentatively established way to solve this kind of problem is to
> > make
> > the assertion return a copy of the value it is asserting a
> > predicate
> > about. Something like this:
> >
> > pp = __buiiltin_assume(p, p != 0);
>
> That ties the meta-intrinsic to its uses, and obfuscates the def-use
> chains. I would not like working with that representation. In
> general,
> I see no reason to move toward program dependence graphs or
> SSI. We can continue the LLVM style of making analyses work with
> SSA+CFG (with dominance). I happen to like that style.
Fair enough, but it is not clear to me that we have the correct side-effect model for this to work. If I have:
void foo(int a, int b) {
if (a > 0) {
__builtin_assume(b == 3);
bar(a, b);
}
bar2(a, b);
}
given that the intrinsic is free (in terms of execution cost) and has no side effects in the usual sense, what prevents some transformation from moving the assumption outside of the if statement? I suppose that we could mark the assumption intrinsic as having unmodeled side effects, but that would be unproductive.
>
> > Then, the predicate would be asserted for pp, but not p, and it
> > would
> > be anchored in the SSA graph, so the dominance properties would be
> > preserved implicitly, rather than requiring explicit dominance
> > queries. The presence of this intrinsic in the SSA graph will
> > require
> > special-casing in passes like instcombine which do
> > pattern-matching,
> > to avoid missing patterns.
>
> Why are we avoiding optimizations that require dominance? Is there
> any
> problem with allowing some set of instcombines to use dominance if
> it's available? This seems crazy not to do in a pass that preserves
> the CFG.
>
> > Note that this approach doesn't automatically eliminate the problem
> > of
> > anchoring the intrinsic with respect to the surrounding control
> > flow
> > and side effects. Presumably, it would still be useful to let the
> > intrinsic appear to have side effects to achieve this, as it does
> > in
> > the current patch. That will interfere with GVN, DSE, LICM, and
> > many
> > other optimizations, but these problems can be special-cased
> > around.
>
> We need the concept of a meta-intrinsic that doesn't interfere with
> DCE
> or code motion. If the intrinsic expresses an invariant, it would
> apply to
> anything that it dominates, so it implicitly assigns control
> dependence
> to metadata. The meta-intrinsic approach is being used for debug info
> and could be generalized to work well for a number of things that are
> handled poorly today:
>
> It makes more sense than the front-end's "unreachable" trick which
> has
> pass ordering problems and is awkward, innefficient, and
> non-scalable.
>
> It would have been a proper way to implement the "expect" instrinsic,
> which currently needs to be stripped before any other pass runs.
>
> It would handle load range data without special casing it for integer
> load instructions. It would allow the ranges to depend on a
> condition.
>
> It could provide a framework for supporting lifetime and invariant
> markers. We have too many special cases here, and I doubt
> that our optimizations are currently robust in their presence.
>
> > The other big topic to discuss in this patch is ephemeral values.
> > This
> > would be a significant landmark change.
> >
> > Ephemeral values add uses of values to the program which wouldn't
> > otherwise be there. This has its purpose of course, but also its
> > downsides, as it will interfere with use_empty() and hasOneUse()
> > optimizations (much of instcombine, etc.). It could easily be a
> > dickens for indvars and lsr, unless, again, it gets the right
> > special-case handling. It'll need special-casing in several
> > analyses
> > which walk def-use lists. The new EphemeralValues analysis pass
> > will
> > make the special-casing less painful, but it's still a value map
> > that
> > has to be passed around and kept current.
> >
> > There are also optimizations that ephemeral values will break that
> > won't be easily special-casable. For example:
> >
> > *r = something();
> > p = *q;
> > __builtin_assume(p);
> > *r = 0;
> >
> > If we don't know whether q and r alias, the presence of the
> > intrinsic,
> > and thus the ephemeral load, is preventing the first store from
> > being
> > eliminated. Ephemeral values can also hold branches ephemerally
> > live,
> > so they can hold early loop exits ephemerally live, which means
> > they
> > could complicate loop optimization. It's a design principle in LLVM
> > that the main middle-end optimizations work to enable other
> > optimizations. Introducing artificial constructs which hold values
> > and
> > branches and stores live interferes with this in a very
> > philosophical
> > way.
>
> Well said. I appreciate that this is a problem worth solving, but I
> think
> adding ephemeral values to the IR will cause a perpetual headache. We
> need to consider other solutions.
Introducing invariants would enable much more than just this use case. As you probably know, invariants have been an often-requested feature for quite some time, and Chandler suggested approaching the alignment problem using invariants as a way to simultaneously advance both desires.
Doing this seems to have two options:
1. Describe invariants using LLVM IR (which naturally seems to give the ephemeral values problem)
2. Create a new language for describing invariants
Expressing invariants in the IR means that they naturally get transformed and updated along with the values they describe.
As a quick summary, here are some uses that have been mentioned for invariants:
- Implementing pointer-alignment assumptions
- Providing additional constraints to SE (and Polly)
- Transplanting loop-bound information, etc. into outlined functions representing loop bodies (for OpenMP, etc.)
(from Chandler on IRC)
- Declaring that two values are equivalent to avoid redundant loads
- Declaring that one vptr being loaded is equivalent to some constant or some other vptr we may have knowledge about (similar to the previous use, but the goal is indirect call resolution, not load removal)
- Declaring that some predicate is known, such as a pointer being non-null, to delete branches on that predicate
>
> I would like the underlying object to have metadata that captures its
> alignment, and possibly offset relative to an aligned address. I
> don't
> see why this couldn't be done with an intrinsic that takes a metadata
> argument refering to the underlying object's value.
Could you type out an explicit example of what you're proposing? To handle just the alignment case, I'm not sure that we need metadata at all.
Thanks again,
Hal
>
> -Andy
>
> > How big of a practical problem is all this special casing and/or
> > hampered optimization? If this mechanism is being aimed at only a
> > few
> > targeted purposes, probably the gains will outweigh the costs. But,
> > if
> > invariants become widely used (for example, if they're used to
> > describe the special properties of C++ reference types), they could
> > be
> > come significantly more troublesome and it's less clear overall.
> >
> > On the code itself:
> >
> > I assume that asserting false incurs immediate undefined behavior,
> > right? If so, then asserting undef may do the same, so the code in
> > lib/Transforms/Utils/Local.cpp is not as aggressive as it could be.
> > Is
> > this intentional? If so, a comment on the philosophy would be
> > helpful.
> > Also, it's not clear why undef is important to special-case there
> > at
> > all.
> >
> > Dan
> >
> > On Wed, Dec 5, 2012 at 12:43 PM, hfinkel at anl.gov <hfinkel at anl.gov>
> > wrote:
> >> Changed to using @llvm.invariant; added analysis for ephemeral
> >> values.
> >>
> >> http://llvm-reviews.chandlerc.com/D150
> >>
> >> CHANGE SINCE LAST DIFF
> >> http://llvm-reviews.chandlerc.com/D150?vs=384&id=439#toc
> >>
> >> Files:
> >> docs/LangRef.html
> >> include/llvm-c/Transforms/Scalar.h
> >> include/llvm/Analysis/EphemeralValues.h
> >> include/llvm/Analysis/InlineCost.h
> >> include/llvm/Analysis/Passes.h
> >> include/llvm/InitializePasses.h
> >> include/llvm/Intrinsics.td
> >> include/llvm/LinkAllPasses.h
> >> include/llvm/Transforms/Scalar.h
> >> lib/Analysis/Analysis.cpp
> >> lib/Analysis/CMakeLists.txt
> >> lib/Analysis/CodeMetrics.cpp
> >> lib/Analysis/EphemeralValues.cpp
> >> lib/Analysis/InlineCost.cpp
> >> lib/Analysis/ValueTracking.cpp
> >> lib/CodeGen/IntrinsicLowering.cpp
> >> lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
> >> lib/Transforms/IPO/InlineSimple.cpp
> >> lib/Transforms/IPO/PassManagerBuilder.cpp
> >> lib/Transforms/Scalar/ADCE.cpp
> >> lib/Transforms/Scalar/AlignmentInvProp.cpp
> >> lib/Transforms/Scalar/CMakeLists.txt
> >> lib/Transforms/Scalar/Scalar.cpp
> >> lib/Transforms/Utils/Local.cpp
> >> test/Analysis/EphemeralValues/lit.local.cfg
> >> test/Analysis/EphemeralValues/simple.ll
> >> test/Transforms/AlignmentInvProp/lit.local.cfg
> >> test/Transforms/AlignmentInvProp/simple.ll
> >> test/Transforms/Inline/ephemeral.ll
> >>
> >> _______________________________________________
> >> llvm-commits mailing list
> >> llvm-commits at cs.uiuc.edu
> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >>
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
--
Hal Finkel
Postdoctoral Appointee
Leadership Computing Facility
Argonne National Laboratory
More information about the llvm-commits
mailing list