Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

standard library APIs missing noalias on rvalue references #47207

Open Quuxplusone opened 3 years ago

Quuxplusone commented 3 years ago
Bugzilla Link PR48238
Status NEW
Importance P enhancement
Reported by Gonzalo BG (gonzalo.gadeschi@gmail.com)
Reported on 2020-11-20 02:31:16 -0800
Last modified on 2020-11-24 01:20:54 -0800
Version trunk
Hardware PC Linux
CC blitzrakete@gmail.com, dblaikie@gmail.com, dgregor@apple.com, eric@efcs.ca, erik.pilkington@gmail.com, jdoerfert@anl.gov, llvm-bugs@lists.llvm.org, richard-llvm@metafoo.co.uk, stefomeister@gmail.com
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also

According to the wording and notes in [res.on.arguments]/1.3, standard library APIs taking rvalue references arguments can assume that these do not alias.

When looking at: https://godbolt.org/z/G8ejKb

include

int fails(int p) { int&& i = std::move(std::move(p)); return i; } int works(int __restrict p) { return p; }

the last call to std::move in "fails" takes an rvalue reference, and its argument should be noalias, which LLVM attributor pass should be able to propagate up to p.

However, the generated IR is missing no-alias.

I don't think the standard guarantees that rvalue references are always no alias, but the standard library does guarantee that for all its APIs.

The simplest solution I can imagine would be to add a [[std_lib_api]] builtin attribute that adds noalias to all rvalue reference arguments of a function. We'd need to implement the argument, and add it to libc++.

Richard, Eric, thoughts?

To me this wording looks extremely powerful, but from https://cplusplus.github.io/LWG/issue1204 I don't know if this is intended or not.

Quuxplusone commented 3 years ago

Not that it changes the result, but the godbolt example doesn't actually run the attributor since it is not turned on by default yet.

This does: https://godbolt.org/z/TxxvKx

Quuxplusone commented 3 years ago
It was confirmed in the LWG mailing list that for a standard library API `foo`

    void foo(int&& a, int&& b) { }

the behavior of:

    int a = 42;
    foo(move(a), move(a));

is undefined because the rvalue references passed to "foo" alias and that's not
allowed according to [res.on.arguments]/1.3.

---

From a libc++ point-of-view, this probably also means that the standard library
needs to be careful of not accessing any static or global from a function that
could accept that value as an argument, e.g.,

   int b = 42;
   void foo(int&& a) {
      b += a; // UB if "a" aliases "b"
   }
   foo(move(b)); // UB

and standard library users need to be careful as well:

   struct F {
     int* p;
     void operator()(int b) {}
   };
   int test() {
     int a = 42;
     std::async(
       F{&a},
       std::move(a) // "a" aliases "F.p"
     );
   }