[llvm-commits] [PATCH] Invariants (and Assume Aligned) - LLVM

Dan Gohman dan433584 at gmail.com
Wed Dec 5 17:03:24 PST 2012


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);

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.

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.


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.

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
>



More information about the llvm-commits mailing list