[PATCH] D74162: [Inliner] Inlining should honor nobuiltin attributes

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson marked 3 inline comments as done.
tejohnson added inline comments.


================
Comment at: llvm/include/llvm/Analysis/TargetLibraryInfo.h:267
+  /// attributes, which is conservatively correct.
+  bool areInlineCompatible(const TargetLibraryInfo &CalleeTLI) const {
+    BitVector B = OverrideAsUnavailable;
----------------
davidxl wrote:
> This may be bad for performance -- as the inline instance will be optimized differently.
Note that it won't be worse than head, which doesn't restrict the inlines based on nobuiltin attributes at all. We could also just disallow inlining completely between callers/callees with different nobuiltin attributes. But I was concerned that this would degrade performance too much by disallowing inlining in too many cases.


================
Comment at: llvm/test/Transforms/Inline/X86/inline-no-builtin-compatible.ll:6
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
----------------
davidxl wrote:
> Is this test x86 specific?
Actually not, I can move to parent directory.


================
Comment at: llvm/test/Transforms/Inline/X86/inline-no-builtin-compatible.ll:23
+  ret i32 %call
+; CHECK-LABEL: nobuiltinmemcpy
+; CHECK: call i32 (...) @externalfunc()
----------------
davidxl wrote:
> why not directly check-not call?
Yeah that would be better, will change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74162