[PATCH] D76140: [InlineFunction] update attributes during inlining

Johannes Doerfert via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Mar 13 15:40:41 PDT 2020


jdoerfert added a comment.

In D76140#1922417 <https://reviews.llvm.org/D76140#1922417>, @anna wrote:

> hmm. yes, I can see this getting complicated for design and correctness. Thanks for the example @jdoerfert.


These things are unfortunately never as easy as we want them to be, believe me I learned the hard way ;)

> Basically, the usecase I'm interested in is the following:
> 
>   callee(i8* %arg) {
>     %r = call i8* @foo(i8* %arg)
>      ret i8* %r
>   }
> 
> 
> And a follow-on example:
> 
>   callee(i8** %arg) {
>     %r = load i8*, i8** %arg <-- here we can add the !nonnull metadata if callsite for callee has nonnull return attr
>      ret i8* %r
>   }
> 
> 
> The caller is:
> 
>   caller {
>     ...
>     call nonnull i8* @callee(i8** %arg) 
>   }
> 
> 
> AFAICT, this should be fine because the only operations in callee context is the load and return. We are not backward propagating something incorrect into the callee context. W.r.t the caller context, what was true before inlining, remains true after inlining as well.



1. If callee is internal and all call site are non-null we can easily make sure the Attributor catches this (if it does not already).
2. If callee is non-internal or not all call site are non-null we could introduce an internal copy of callee for all "good" call sites (the Attributor issue on this is here: https://github.com/llvm/llvm-project/issues/172)
3. If your use case is 2) but very restricted, you can walk the successors of the call/load and make sure they are all "OK", e.g., use `MustBeExecutedContextExplorer::findInContextOf`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76140/new/

https://reviews.llvm.org/D76140





More information about the llvm-commits mailing list