[PATCH] D92657: Fix interaction between clang and some inline builtins from glibc under _FORTIFY_SOURCE

[Reid Kleckner via Phabricator via llvm-commits]

rnk requested changes to this revision.
rnk added a subscriber: rjmccall.
rnk added inline comments.
This revision now requires changes to proceed.


================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:39
 #include "clang/AST/StmtVisitor.h"
+#include "clang/Analysis/Analyses/Dominators.h"
+#include "clang/Analysis/CFGStmtMap.h"
----------------
I see a dep from clangCodeGen to clangAnalysis in the CMakeLists, but this is the first include of clang/Analysis/* from CodeGen. I think we should consider this new dependency more carefully.

@rjmccall, can you please advise about the new dependency?


================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2984-2986
+    std::unique_ptr<CFG> cfg =
+        CFG::buildCFG(FD, Body, &Context, CFG::BuildOptions());
+    ParentMap PM(Body);
----------------
These are both really expensive. Is this really necessary?


================
Comment at: clang/test/CodeGen/memmove-always-inline-definition-used.c:1
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -S -emit-llvm -o - %s | FileCheck %s
+//
----------------
I'd like to see the generated IR before LLVM optimization passes run. Please add a RUN line with -disable-llvm-passes and add checks that show what attributes get applied where. From the code, I think `noinline` goes on the call site, but I wanted to check my understanding in the test.


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

https://reviews.llvm.org/D92657