samitolvanen added inline comments.

================
Comment at: clang/test/CodeGen/builtin-addressof-nocfi.c:18
+  // CHECK: call void @c(void ()* no_cfi @a)
+  c(__builtin_addressof_nocfi(a));
+  e();
----------------
nickdesaulniers wrote:
> do we ever need the builtin address of a global that's NOT a function?
> 
> If so, we should add a test for that. If not, perhaps we don't need to waste 
> space in every APValue?
> do we ever need the builtin address of a global that's NOT a function?

I don't think so. This version does accept any global because it was modelled 
after `__builtin_addressof()`, but we could look into limiting this only for 
functions. Perhaps the built-in name should also be changed in that case?

> If so, we should add a test for that. If not, perhaps we don't need to waste 
> space in every APValue?

What would be a better place for the flag? I think in Clang, changing this to 
support only functions would probably just need some additional Sema checks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108479

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to