llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
27.82k stars 11.45k forks source link

clang/llvm looses restrictness, resulting in wrong code #38588

Open dobbelaj-snps opened 5 years ago

dobbelaj-snps commented 5 years ago
Bugzilla Link 39240
Version trunk
OS Windows NT
CC @hfinkel,@viccpp

Extended Description

clang/llvm implements argument restrictness by putting a noalias on the the restrict argument. When alias analysis traces the base object of a pointer to two different base objects of which one is marked with noalias, it concludes that these pointers do not alias.

This mapping results in wrong behavior according to the restrict specification: (or my interpretation of it is wrong ;) ):

6.7.3.1, 4: 'every other lvalue used to access the value of X shall also have its address based on P.'

where P is the restrict annotated pointer. This means that in following code tmp and pA might alias:

  // Assume pA and pB point to the same set of objects X
  // restrict rules dictate that all accesses in the scope must happen through
  // the restrict pointer (pA) or a pointer based on it.
  int foo(int* __restrict pA, int* pB, int* pC) {
    int *tmp=pA; // tmp is based on pA 
    pA=pB; // pB also points into the set of objects X, but, accessing it must happen through a pointer based on pA

    *tmp=42; // tmp points into the set of objects X
    *pA=43;  // pA points into the set of objects X
    *pC=99;  // pC does not point into the set of objects X
    return *tmp; // fail: needs a load !!! (either 42 or 43)
  }

results in:

  define dso_local i32 @foo(i32* noalias nocapture, i32* nocapture, i32* nocapture) local_unnamed_addr #0 {
    store i32 42, i32* %0, align 4, !tbaa !2
    store i32 43, i32* %1, align 4, !tbaa !2
    store i32 99, i32* %2, align 4, !tbaa !2
    ret i32 42 ; WRONG! %0 must be reloaded
  }

But, clang/llvm assumes that tmp and pA will never alias, and propagates the 42.

NOTES:

hfinkel commented 5 years ago
  • llvm + Hal Finkels local restrict patches has the correct behavior, IF the restrict is made local

Interesting test case. We could easily change this to also work for the function-argument case, and it looks like we may need to do just that.