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

[Chuanqi Xu via Phabricator via llvm-commits]

ChuanqiXu accepted this revision.
ChuanqiXu added inline comments.
This revision is now accepted and ready to land.


================
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]]
----------------
alexander-shaposhnikov wrote:
> 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) 
OK, LGTM then. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131872