[PATCH] D108479: [Clang] Add __builtin_addressof_nocfi

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.


================
Comment at: clang/lib/CodeGen/CGExprConstant.cpp:1890
 
-    if (auto FD = dyn_cast<FunctionDecl>(D))
-      return CGM.GetAddrOfFunction(FD);
+    if (auto FD = dyn_cast<FunctionDecl>(D)) {
+      auto *C = CGM.GetAddrOfFunction(FD);
----------------
fix linter warning


================
Comment at: clang/lib/CodeGen/CGExprConstant.cpp:1891
+    if (auto FD = dyn_cast<FunctionDecl>(D)) {
+      auto *C = CGM.GetAddrOfFunction(FD);
+      if (base.isNoCFIValue())
----------------
Don't use `auto` here.
https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable
It's common when the type is already specified on the RHS of an assignment that's using of the casting operators that's already explicitly specialized with the type.


================
Comment at: clang/test/CodeGen/builtin-addressof-nocfi.c:18
+  // CHECK: call void @c(void ()* no_cfi @a)
+  c(__builtin_addressof_nocfi(a));
+  e();
----------------
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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108479