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 cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits