[PATCH] D131872: [Intrinsics] Add initial support for NonNull attribute

[Alexander Shaposhnikov via Phabricator via llvm-commits]

alexander-shaposhnikov marked 2 inline comments as done.
alexander-shaposhnikov added inline comments.


================
Comment at: clang/test/CodeGenCXX/threadlocal_address.cpp:27
 // CHECK-O1-NEXT: entry:
-// CHECK-O1-NEXT:   %[[I_ADDR:.+]] = {{.*}}call ptr @llvm.threadlocal.address.p0(ptr nonnull @i)
+// CHECK-O1-NEXT:   %[[I_ADDR:.+]] = {{.*}}call ptr @llvm.threadlocal.address.p0(ptr @i)
 // CHECK-O1-NEXT:   %[[VAL:.+]] = load i32, ptr %[[I_ADDR]]
----------------
ChuanqiXu wrote:
> Why do we move the `nonnull` specifier here? And it looks like we need a nonnull attribute for the return value. (Same for other tests.)
Previously (without this patch) this nonnull attribute was added by InstCombinePass 
(at the call site) (https://github.com/llvm/llvm-project/blob/main/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp#L2949 ),
this patch annotates llvm.threadlocal.address.p0 itself (see the check on line 23) thus adding the attribute at the call site becomes unnecessary (and InstCombine skips it: https://github.com/llvm/llvm-project/blob/main/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp#L2938) 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131872