nickdesaulniers added a comment.
Thanks for the patch!
Please fix the lint checks. `git-clang-format HEAD~` should help, and IIRC
there is a git hook when using `arc diff` (though maybe that requires one time
setup? I seem to have an `.arclint` file in my `llvm-projects` checkout.
================
Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:1306
+ Fn->setName(Fn->getName() + ".inline");
+ }
+
----------------
avoid `{}` for single statement conditionals.
https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements
================
Comment at: clang/test/CodeGen/memcpy-inline-builtin.c:1
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -S -emit-llvm -o - %s
-disable-llvm-passes | FileCheck %s
+//
----------------
Would `-emit-codegen-only` be more appropriate here than `-disable-llvm-passes`?
================
Comment at: clang/test/CodeGen/memcpy-inline-builtin.c:7
+
+// always_inline is used so clang will emit this body. Otherwise, we need >=
-O1.
+#define AVAILABLE_EXTERNALLY extern inline __attribute__((always_inline)) \
----------------
I'm not sure what `this` refers to in the comment?
Also, this whole bit about `always_inline` smells fishy to me. The semantics
of `gnu_inline` is that no body is emitted. If you //don't// want a body
//don't// use `gnu_inline`. Or was this set of qualifiers+fn attrs taken
directly from the kernel, or glibc? `FunctionDecl::isInlineBuiltinDeclaration`
only checks for builtin ID, has body, and whether inline was specified.
================
Comment at: clang/test/CodeGen/memcpy-inline-builtin.c:20
+void foo(void *a, const void *b, size_t c) {
+ // CHECK: call i8* @memcpy.inline
+ memcpy(a, b, c);
----------------
Can you use `llvm/utils/update_cc_test_checks.py` to autogen full checks for
this file? I think it would be helpful to reviewers to be able to see
everything that's generated here, even if we only really care that the `call`
is to the `.inline` suffixed version.
================
Comment at: clang/test/CodeGen/pr9614.c:44
// CHECK: declare i8* @memchr(
+// CHECK: declare i32 @abs(i32
// CHECK: declare void @llvm.prefetch.p0i8(
----------------
Can you elaborate on these changes? Surprising.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D109967/new/
https://reviews.llvm.org/D109967
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
