[PATCH] D93927: [ArgPromotion] Copy !range metadata for loads.

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added a comment.

In D93927#2652456 <https://reviews.llvm.org/D93927#2652456>, @tejohnson wrote:

> This is causing a runtime failure in one of our tests. Specifically due to the nonnull metadata. It appears that this should not be cloned because the circumstances under which it is valid and applied to the load in the callee may not exist in the caller. I'm not sure about whether this is an issue for some of the other metadata. I'm going to try to create a smaller test case. But perhaps this change should be reverted.

Here is a small test case. Note I have to split the invocation into 2 to get the problem to reproduce, due to phase ordering between InstCombine Calls that creates the !nonnull metadata and ArgPromotion that propagates it during promotion. We are hitting this in a ThinLTO build, where the !nonnull metadata is added during the initial compile, and after some additional inlining in the ThinLTO post link pipeline it then gets the argument promotion.

I am not sure if the other attributes being propagated could also hit issues like this. Can you revert while it is being investigated?

  $ cat c.cc
  struct Bar {
    int i;
  };
  struct Foo final {
    Bar *b;
    bool x;
  };
  
  __attribute__((noinline)) static int func(const Foo &f) {
    // This initially looked like:
    // __builtin_assume(!f.x || f.b != nullptr);
    // and in the original test case jump threading transformed it into the form
    // below with the assume split. Since I couldn't get jump threading to do this
    // for my small test case I have manually made the transformation here.
    if (!f.x)
      __builtin_assume(true);
    else
      __builtin_assume(f.b != nullptr);
    if (f.x)
      // Because this is essentially under the same condition as the
      // __builtin_assume(f.b != nullptr), the load of f.b gets the
      // !nonnull metadata.
      return f.b->i;
    return 0;
  }
  
  int caller(const Foo &f) {
    return func(f);
  }
  
  $ clang++ -O1 c.cc -flto=thin -S
  # c.s has !nonnull on dereference in func()
  $ opt < c.s -argpromotion -S
  # !nonnull has been propagated to dereference now in caller(). However, this load is no longer guarded by the check of "if (f.x)"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93927