nickdesaulniers added subscribers: aaron.ballman, rsmith.
nickdesaulniers added inline comments.
================
Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:1302
// in case the function pointer is referenced somewhere.
- if (FD->isInlineBuiltinDeclaration() && Fn) {
+ if (Fn) {
std::string FDInlineName = (Fn->getName() + ".inline").str();
----------------
I don't think we want to do all this work if just `Fn`; ie. create a new
`std::string` with `.inline` suffix for every function we're going to generate
code (IR) for. How about we add an additional unlikely guard:
`if (FD->getBuiltinID() && FN) {`
Because in the usual case, `FD` both has a builtin ID and is an inline builtin
declaration, while in the exceptional case that this patch addresses, `FD` has
a builtin ID but is not an inline builtin declaration.
================
Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:1319-1321
+ // everywhere. That's GCC behavior too. Unfortunately, I cannot find a way
+ // to detect that situation before we reach codegen, so do some late
+ // replacement.
----------------
Perhaps in `Sema::CheckFunctionDeclaration`? I see there is where we detect
redeclarations. The calls from there to
`FunctionDecl::setPreviousDeclaration()` seem to set up the redecl chain.
Perhaps this exceptional case (or both cases, even) would be handled better
there?
cc @rsmith @aaron.ballman in case they have feedback/tips/cycles.
================
Comment at: clang/test/CodeGen/strlen-inline-builtin-redecl.c:8
+unsigned long strnlen(const char *, unsigned long);
+void fortify_panic(const char *);
+
----------------
unused decl
================
Comment at: clang/test/CodeGen/strlen-inline-builtin-redecl.c:10
+
+extern inline __attribute__((always_inline)) __attribute__((gnu_inline))
unsigned long strlen(const char *p) {
+ return 1;
----------------
Do you mind wrapping this to 80 chars wide? I suspect if you put the two
`__attribute__`s first, then the formatter will do a better job. You can also
combine these, a la `__attribute__((always_inline, gnu_inline))` to cut down on
line length.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D112059/new/
https://reviews.llvm.org/D112059
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
