[PATCH] D150396: [InlineCost] Check for conflicting target attributes early

[Kazu Hirata via Phabricator via llvm-commits]

kazu updated this revision to Diff 527106.
kazu added a comment.

Add a FIXME comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150396

Files:
  llvm/lib/Analysis/InlineCost.cpp
  llvm/test/Transforms/Inline/target-features-vs-alwaysinline.ll


Index: llvm/test/Transforms/Inline/target-features-vs-alwaysinline.ll
===================================================================
--- /dev/null
+++ llvm/test/Transforms/Inline/target-features-vs-alwaysinline.ll
@@ -0,0 +1,36 @@
+; RUN: opt < %s -passes=inline -pass-remarks-missed=inline -S 2>&1 | FileCheck %s
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Make sure that we do not inline callee into caller.  If we inline
+; callee into caller, caller would end pu with AVX512 intrinsics even
+; though it is not allowed to use AVX512 instructions.
+; CHECK: remark: [[MSG:.*]] because it should never be inlined (cost=never): conflicting target attributes
+
+define void @caller(ptr %0) {
+; CHECK-LABEL: define void @caller
+; CHECK-SAME: (ptr [[TMP0:%.*]]) {
+; CHECK-NEXT:    call void @callee(ptr [[TMP0]], i64 0, i32 0) #[[ATTR2:[0-9]+]]
+; CHECK-NEXT:    ret void
+;
+  call void @callee(ptr %0, i64 0, i32 0) #1
+  ret void
+}
+
+define available_externally void @callee(ptr %0, i64 %1, i32 %2) #0 {
+; CHECK-LABEL: define available_externally void @callee
+; CHECK-SAME: (ptr [[TMP0:%.*]], i64 [[TMP1:%.*]], i32 [[TMP2:%.*]]) #[[ATTR0:[0-9]+]] {
+; CHECK-NEXT:    [[TMP4:%.*]] = call <16 x float> @llvm.x86.avx512.min.ps.512(<16 x float> zeroinitializer, <16 x float> zeroinitializer, i32 0)
+; CHECK-NEXT:    store <16 x float> [[TMP4]], ptr [[TMP0]], align 1
+; CHECK-NEXT:    ret void
+;
+  %4 = call <16 x float> @llvm.x86.avx512.min.ps.512(<16 x float> zeroinitializer, <16 x float> zeroinitializer, i32 0)
+  store <16 x float> %4, ptr %0, align 1
+  ret void
+}
+
+declare <16 x float> @llvm.x86.avx512.min.ps.512(<16 x float>, <16 x float>, i32 immarg)
+
+attributes #0 = { "target-features"="+aes,+avx,+avx2,+avx512bw,+avx512dq,+avx512f,+avx512vl,+bmi,+bmi2,+crc32,+cx16,+cx8,+f16c,+fma,+fsgsbase,+fxsr,+invpcid,+lzcnt,+mmx,+movbe,+pclmul,+popcnt,+rdrnd,+sahf,+sse,+sse2,+sse3,+sse4.1,+sse4.2,+ssse3,+x87,+xsave,+xsaveopt" }
+attributes #1 = { alwaysinline }
Index: llvm/lib/Analysis/InlineCost.cpp
===================================================================
--- llvm/lib/Analysis/InlineCost.cpp
+++ llvm/lib/Analysis/InlineCost.cpp
@@ -2801,16 +2801,14 @@
 /// Test that there are no attribute conflicts between Caller and Callee
 ///        that prevent inlining.
 static bool functionsHaveCompatibleAttributes(
-    Function *Caller, Function *Callee, TargetTransformInfo &TTI,
+    Function *Caller, Function *Callee,
     function_ref<const TargetLibraryInfo &(Function &)> &GetTLI) {
   // Note that CalleeTLI must be a copy not a reference. The legacy pass manager
   // caches the most recently created TLI in the TargetLibraryInfoWrapperPass
   // object, and always returns the same object (which is overwritten on each
   // GetTLI call). Therefore we copy the first result.
   auto CalleeTLI = GetTLI(*Callee);
-  return (IgnoreTTIInlineCompatible ||
-          TTI.areInlineCompatible(Caller, Callee)) &&
-         GetTLI(*Caller).areInlineCompatible(CalleeTLI,
+  return GetTLI(*Caller).areInlineCompatible(CalleeTLI,
                                              InlineCallerSupersetNoBuiltin) &&
          AttributeFuncs::areInlineCompatible(*Caller, *Callee);
 }
@@ -2926,6 +2924,12 @@
                                      " address space");
     }
 
+  // Never inline functions with conflicting target attributes.
+  Function *Caller = Call.getCaller();
+  if (!IgnoreTTIInlineCompatible &&
+      !CalleeTTI.areInlineCompatible(Caller, Callee))
+    return InlineResult::failure("conflicting target attributes");
+
   // Calls to functions with always-inline attributes should be inlined
   // whenever possible.
   if (Call.hasFnAttr(Attribute::AlwaysInline)) {
@@ -2940,8 +2944,12 @@
 
   // Never inline functions with conflicting attributes (unless callee has
   // always-inline attribute).
-  Function *Caller = Call.getCaller();
-  if (!functionsHaveCompatibleAttributes(Caller, Callee, CalleeTTI, GetTLI))
+  // FIXME: functionsHaveCompatibleAttributes below checks for compatibilities
+  // of different kinds of function attributes -- sanitizer-related ones,
+  // checkDenormMode, no-builtin-memcpy, etc.  It's unclear if we really want
+  // the always-inline attribute to take precedence over these different types
+  // of function attributes.
+  if (!functionsHaveCompatibleAttributes(Caller, Callee, GetTLI))
     return InlineResult::failure("conflicting attributes");
 
   // Don't inline this call if the caller has the optnone attribute.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D150396.527106.patch
Type: text/x-patch
Size: 4613 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230531/001f1fb0/attachment.bin>