From d6f994acb3d545b80161e24ab742c9c69d4bbf33 Mon Sep 17 00:00:00 2001 From: Kazu Hirata Date: Fri, 2 Jun 2023 16:00:47 -0700 Subject: [PATCH] [InlineCost] Check for conflicting target attributes early When we inline a callee into a caller, the compiler needs to make sure that the caller supports a superset of instruction sets that the callee is allowed to use. Normally, we check for the compatibility of target features via functionsHaveCompatibleAttributes, but that happens after we decide to honor call site attribute Attribute::AlwaysInline. If the caller contains a call marked with Attribute::AlwaysInline, which can happen with __attribute__((flatten)) placed on the caller, the caller could end up with code that cannot be lowered to assembly code. This patch fixes the problem by checking the target feature compatibility before we honor Attribute::AlwaysInline. Fixes https://github.com/llvm/llvm-project/issues/62664 Differential Revision: https://reviews.llvm.org/D150396 --- llvm/lib/Analysis/InlineCost.cpp | 20 +++++++---- .../Inline/target-features-vs-alwaysinline.ll | 36 +++++++++++++++++++ 2 files changed, 50 insertions(+), 6 deletions(-) create mode 100644 llvm/test/Transforms/Inline/target-features-vs-alwaysinline.ll diff --git a/llvm/lib/Analysis/InlineCost.cpp b/llvm/lib/Analysis/InlineCost.cpp index 02871aa358fa..baf4185f2407 100644 --- a/llvm/lib/Analysis/InlineCost.cpp +++ b/llvm/lib/Analysis/InlineCost.cpp @@ -2801,16 +2801,14 @@ LLVM_DUMP_METHOD void InlineCostCallAnalyzer::dump() { print(dbgs()); } /// 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 &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 @@ std::optional llvm::getAttributeBasedInliningDecision( " 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 @@ std::optional llvm::getAttributeBasedInliningDecision( // 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. diff --git a/llvm/test/Transforms/Inline/target-features-vs-alwaysinline.ll b/llvm/test/Transforms/Inline/target-features-vs-alwaysinline.ll new file mode 100644 index 000000000000..03c6df76f529 --- /dev/null +++ b/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 }