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

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

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

> In D76140#1922292 <https://reviews.llvm.org/D76140#1922292>, @reames wrote:
>
> > In D76140#1922162 <https://reviews.llvm.org/D76140#1922162>, @jdoerfert wrote:
> >
> > > In addition to @reames correctness concerns this is not valid for a different reason. You cannot backwards propagate information in the general case:
> > >
> > >   __attribute__((nonnull)) int *foo(int c) {
> > >      int *A = unknown();
> > >      int *B = unknown();
> > >      do_sth_with_ptr_and_may_use_nonnull(A, B);
> > >      return c ? A : B; // or if (C) return A; else return B;
> > >   }
> > >
> > >
> > > Knowing `foo` returns a `nonnull` value cannot be used to annotate `A` or `B`.
> >
> >
> > Good catch, I'd missed that.
>
>
> Just noticed this. I think we can special case this to: the return and def call are in the same basic block (it will take care of the if-else in the example) and has only one use which is the return. The latter takes care of avoiding incorrect semantics in that call `do_sth_with_ptr_and_may_use_nonnull`  or any other use that depends on knowing which of the ptr is non-null.


None of this is sufficient. We are repeating a lot of deduction logic here now...

Take:

  __attribute__((nonnull)) int *foo() {
      int *Base = unknown();
      do_sth_with_ptr_and_may_use_nonnull(Base);
      int *A = return_arg(/* returned */ Base);
      exit();
      return A;
  }

Return and call are in the same block, only a single use of A exists, however, backwards propagating `nonnull` to A is wrong and will be problematic if it is used to optimize Base.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76140