[PATCH] D22978: [CFLAA] Revert all modref supports due to their buggy nature

[Daniel Berlin via llvm-commits]

On Fri, Jul 29, 2016 at 2:51 PM, Jia Chen via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> grievejia created this revision.
> grievejia added reviewers: george.burgess.iv, hfinkel.
> grievejia added a subscriber: llvm-commits.
>
> Unfortunately, the getModRefBehavior() functions for both CFL-AA turn out
> to be very buggy.
>
> Currently, getModRefBehavior() is implemented by looking at
> AliasSummary.RetParamRelations, a vector that records how **pointer
> values** are accessed. Non-pointer values are simply ignored. As a result,
> if inside the function we store an i32 to one of the i32* parameter, or if
> we load an i64 from one of the i64* parameter, the load/store won't be
> recorded by RetParamRelations since i32 and i64 are scalar types, not
> pointer types.





> If we only check RetParamRelations for modref behavior, we will inevitably
> miss those scalar loads and stores. The takeaway point here is that
> RetParamRelations is good for summarizing pointer aliases, but cannot be
> relied on to summarize function modref behaviors.
>
>


> Another problem with our current getModRefBehavior() implementation is
> that when analyzing modref for function f, it does not consider any
> functions that are called by f. For example, if f() is a function that
> calls certain library function with side-effects, those side effects won't
> be aggregated into f's modref behavior, which is problematic.
>
> The lesson I learned from debugging those problems is that modref and
> aliasing are conceptually two different problems.


yes.


> Aliasing can be used to derive modrefs, but they're still two separate
> analyses and conflating the two into a single pass doesn't sound like a
> good idea.


You should not try to answer modref from points-to sets alone, yes ;)
They are used as basis sets to compute mod ref info.

Note that mod-ref can be solved as a reachability problem, but not sure
that makes the most sense either.


> I'd say that let's remove the buggy implementation for now until we can
> come up with a more principled, less error-prone approach .
>
> https://reviews.llvm.org/D22978
>
> Files:
>   include/llvm/Analysis/CFLAndersAliasAnalysis.h
>   include/llvm/Analysis/CFLSteensAliasAnalysis.h
>   lib/Analysis/CFLAndersAliasAnalysis.cpp
>   lib/Analysis/CFLSteensAliasAnalysis.cpp
>   test/Analysis/CFLAliasAnalysis/Andersen/interproc-ret-arg.ll
>
> test/Analysis/CFLAliasAnalysis/Andersen/interproc-ret-deref-arg-multilevel.ll
>   test/Analysis/CFLAliasAnalysis/Andersen/interproc-ret-deref-arg.ll
>
> test/Analysis/CFLAliasAnalysis/Andersen/interproc-ret-ref-arg-multilevel.ll
>   test/Analysis/CFLAliasAnalysis/Andersen/interproc-ret-ref-arg.ll
>   test/Analysis/CFLAliasAnalysis/Andersen/interproc-store-arg.ll
>   test/Analysis/CFLAliasAnalysis/Steensgaard/interproc-ret-arg.ll
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160801/e256fe5c/attachment-0001.html>