Skip to content

Commit 2b2d56f

Browse files
echristoechristo
committed
Provide a frontend based error for always_inline functions that require
target features that the caller function doesn't provide. This matches the existing backend failure to inline functions that don't have matching target features - and diagnoses earlier in the case of always_inline. Fix up a few test cases that were, in fact, invalid if you tried to generate code from the backend with the specified target features and add a couple of tests to illustrate what's going on. This should fix PR25246. llvm-svn: 252834
1 parent fbfd97e commit 2b2d56f

File tree

7 files changed

+94
-123
lines changed

7 files changed

+94
-123
lines changed

‎clang/include/clang/Basic/DiagnosticSemaKinds.td

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -431,6 +431,9 @@ def err_builtin_definition : Error<"definition of builtin function %0">;
431431
def err_arm_invalid_specialreg : Error<"invalid special register for builtin">;
432432
def err_invalid_cpu_supports : Error<"invalid cpu feature string for builtin">;
433433
def err_builtin_needs_feature : Error<"%0 needs target feature %1">;
434+
def err_function_needs_feature
435+
: Error<"function %0 and always_inline callee function %1 are required to "
436+
"have matching target features">;
434437
def warn_builtin_unknown : Warning<"use of unknown builtin %0">,
435438
InGroup<ImplicitFunctionDeclare>, DefaultError;
436439
def warn_dyn_class_memaccess : Warning<

‎clang/lib/CodeGen/CGExpr.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3747,6 +3747,15 @@ RValue CodeGenFunction::EmitCall(QualType CalleeType, llvm::Value *Callee,
37473747
assert(CalleeType->isFunctionPointerType() &&
37483748
"Call must have function pointer type!");
37493749

3750+
if (const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(TargetDecl))
3751+
// If this isn't an always_inline function we can't guarantee that any
3752+
// function isn't being used correctly so only check if we have the
3753+
// attribute and a set of target attributes that might be different from
3754+
// our default.
3755+
if (TargetDecl->hasAttr<AlwaysInlineAttr>() &&
3756+
TargetDecl->hasAttr<TargetAttr>())
3757+
checkTargetFeatures(E, FD);
3758+
37503759
CalleeType = getContext().getCanonicalType(CalleeType);
37513760

37523761
const auto *FnType =

‎clang/lib/CodeGen/CodeGenFunction.cpp

Lines changed: 65 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1843,7 +1843,8 @@ template void CGBuilderInserter<PreserveNames>::InsertHelper(
18431843
llvm::BasicBlock::iterator InsertPt) const;
18441844
#undef PreserveNames
18451845

1846-
// Returns true if we have a valid set of target features.
1846+
// Emits an error if we don't have a valid set of target features for the
1847+
// called function.
18471848
void CodeGenFunction::checkTargetFeatures(const CallExpr *E,
18481849
const FunctionDecl *TargetDecl) {
18491850
// Early exit if this is an indirect call.
@@ -1856,31 +1857,70 @@ void CodeGenFunction::checkTargetFeatures(const CallExpr *E,
18561857
if (!FD)
18571858
return;
18581859

1860+
// Grab the required features for the call. For a builtin this is listed in
1861+
// the td file with the default cpu, for an always_inline function this is any
1862+
// listed cpu and any listed features.
18591863
unsigned BuiltinID = TargetDecl->getBuiltinID();
1860-
const char *FeatureList =
1861-
CGM.getContext().BuiltinInfo.getRequiredFeatures(BuiltinID);
1864+
if (BuiltinID) {
1865+
SmallVector<StringRef, 1> ReqFeatures;
1866+
const char *FeatureList =
1867+
CGM.getContext().BuiltinInfo.getRequiredFeatures(BuiltinID);
1868+
// Return if the builtin doesn't have any required features.
1869+
if (!FeatureList || StringRef(FeatureList) == "")
1870+
return;
1871+
StringRef(FeatureList).split(ReqFeatures, ",");
18621872

1863-
if (!FeatureList || StringRef(FeatureList) == "")
1864-
return;
1873+
// If there aren't any required features listed then go ahead and return.
1874+
if (ReqFeatures.empty())
1875+
return;
18651876

1866-
llvm::StringMap<bool> FeatureMap;
1867-
CGM.getFunctionFeatureMap(FeatureMap, FD);
1868-
1869-
// If we have at least one of the features in the feature list return
1870-
// true, otherwise return false.
1871-
SmallVector<StringRef, 1> AttrFeatures;
1872-
StringRef(FeatureList).split(AttrFeatures, ",");
1873-
if (!std::all_of(AttrFeatures.begin(), AttrFeatures.end(),
1874-
[&](StringRef &Feature) {
1875-
SmallVector<StringRef, 1> OrFeatures;
1876-
Feature.split(OrFeatures, "|");
1877-
return std::any_of(OrFeatures.begin(), OrFeatures.end(),
1878-
[&](StringRef &Feature) {
1879-
return FeatureMap[Feature];
1880-
});
1881-
}))
1882-
CGM.getDiags().Report(E->getLocStart(), diag::err_builtin_needs_feature)
1883-
<< TargetDecl->getDeclName()
1884-
<< CGM.getContext().BuiltinInfo.getRequiredFeatures(BuiltinID);
1885-
}
1877+
// Now build up the set of caller features and verify that all the required
1878+
// features are there.
1879+
llvm::StringMap<bool> CallerFeatureMap;
1880+
CGM.getFunctionFeatureMap(CallerFeatureMap, FD);
1881+
1882+
// If we have at least one of the features in the feature list return
1883+
// true, otherwise return false.
1884+
if (!std::all_of(
1885+
ReqFeatures.begin(), ReqFeatures.end(), [&](StringRef &Feature) {
1886+
SmallVector<StringRef, 1> OrFeatures;
1887+
Feature.split(OrFeatures, "|");
1888+
return std::any_of(OrFeatures.begin(), OrFeatures.end(),
1889+
[&](StringRef &Feature) {
1890+
return CallerFeatureMap.lookup(Feature);
1891+
});
1892+
}))
1893+
CGM.getDiags().Report(E->getLocStart(), diag::err_builtin_needs_feature)
1894+
<< TargetDecl->getDeclName()
1895+
<< CGM.getContext().BuiltinInfo.getRequiredFeatures(BuiltinID);
1896+
1897+
} else if (TargetDecl->hasAttr<TargetAttr>()) {
1898+
// Get the required features for the callee.
1899+
SmallVector<StringRef, 1> ReqFeatures;
1900+
llvm::StringMap<bool> CalleeFeatureMap;
1901+
CGM.getFunctionFeatureMap(CalleeFeatureMap, TargetDecl);
1902+
for (const auto &F : CalleeFeatureMap)
1903+
ReqFeatures.push_back(F.getKey());
1904+
// If there aren't any required features listed then go ahead and return.
1905+
if (ReqFeatures.empty())
1906+
return;
18861907

1908+
// Now get the features that the caller provides.
1909+
llvm::StringMap<bool> CallerFeatureMap;
1910+
CGM.getFunctionFeatureMap(CallerFeatureMap, FD);
1911+
1912+
// If we have at least one of the features in the feature list return
1913+
// true, otherwise return false.
1914+
if (!std::all_of(
1915+
ReqFeatures.begin(), ReqFeatures.end(), [&](StringRef &Feature) {
1916+
SmallVector<StringRef, 1> OrFeatures;
1917+
Feature.split(OrFeatures, "|");
1918+
return std::any_of(OrFeatures.begin(), OrFeatures.end(),
1919+
[&](StringRef &Feature) {
1920+
return CallerFeatureMap.lookup(Feature);
1921+
});
1922+
}))
1923+
CGM.getDiags().Report(E->getLocStart(), diag::err_function_needs_feature)
1924+
<< FD->getDeclName() << TargetDecl->getDeclName();
1925+
}
1926+
}

‎clang/test/CodeGen/3dnow-builtins.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// REQUIRES: x86-registered-target
2-
// RUN: %clang_cc1 %s -triple=x86_64-unknown-unknown -target-feature +3dnow -emit-llvm -o - -Werror | FileCheck %s
3-
// RUN: %clang_cc1 %s -triple=x86_64-unknown-unknown -target-feature +3dnow -S -o - -Werror | FileCheck %s --check-prefix=CHECK-ASM
2+
// RUN: %clang_cc1 %s -triple=x86_64-unknown-unknown -target-feature +3dnowa -emit-llvm -o - -Werror | FileCheck %s
3+
// RUN: %clang_cc1 %s -triple=x86_64-unknown-unknown -target-feature +3dnowa -S -o - -Werror | FileCheck %s --check-prefix=CHECK-ASM
44

55
// Don't include mm_malloc.h, it's system specific.
66
#define __MM_MALLOC_H

‎clang/test/CodeGen/avx512vl-builtins.c

Lines changed: 0 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -5,102 +5,6 @@
55

66
#include <immintrin.h>
77

8-
__mmask8 test_mm256_cmpeq_epi32_mask(__m256i __a, __m256i __b) {
9-
// CHECK-LABEL: @test_mm256_cmpeq_epi32_mask
10-
// CHECK: @llvm.x86.avx512.mask.pcmpeq.d.256
11-
return (__mmask8)_mm256_cmpeq_epi32_mask(__a, __b);
12-
}
13-
14-
__mmask8 test_mm256_mask_cmpeq_epi32_mask(__mmask8 __u, __m256i __a, __m256i __b) {
15-
// CHECK-LABEL: @test_mm256_mask_cmpeq_epi32_mask
16-
// CHECK: @llvm.x86.avx512.mask.pcmpeq.d.256
17-
return (__mmask8)_mm256_mask_cmpeq_epi32_mask(__u, __a, __b);
18-
}
19-
20-
__mmask8 test_mm_cmpeq_epi32_mask(__m128i __a, __m128i __b) {
21-
// CHECK-LABEL: @test_mm_cmpeq_epi32_mask
22-
// CHECK: @llvm.x86.avx512.mask.pcmpeq.d.128
23-
return (__mmask8)_mm_cmpeq_epi32_mask(__a, __b);
24-
}
25-
26-
__mmask8 test_mm_mask_cmpeq_epi32_mask(__mmask8 __u, __m128i __a, __m128i __b) {
27-
// CHECK-LABEL: @test_mm_mask_cmpeq_epi32_mask
28-
// CHECK: @llvm.x86.avx512.mask.pcmpeq.d.128
29-
return (__mmask8)_mm_mask_cmpeq_epi32_mask(__u, __a, __b);
30-
}
31-
32-
__mmask8 test_mm256_cmpeq_epi64_mask(__m256i __a, __m256i __b) {
33-
// CHECK-LABEL: @test_mm256_cmpeq_epi64_mask
34-
// CHECK: @llvm.x86.avx512.mask.pcmpeq.q.256
35-
return (__mmask8)_mm256_cmpeq_epi64_mask(__a, __b);
36-
}
37-
38-
__mmask8 test_mm256_mask_cmpeq_epi64_mask(__mmask8 __u, __m256i __a, __m256i __b) {
39-
// CHECK-LABEL: @test_mm256_mask_cmpeq_epi64_mask
40-
// CHECK: @llvm.x86.avx512.mask.pcmpeq.q.256
41-
return (__mmask8)_mm256_mask_cmpeq_epi64_mask(__u, __a, __b);
42-
}
43-
44-
__mmask8 test_mm_cmpeq_epi64_mask(__m128i __a, __m128i __b) {
45-
// CHECK-LABEL: @test_mm_cmpeq_epi64_mask
46-
// CHECK: @llvm.x86.avx512.mask.pcmpeq.q.128
47-
return (__mmask8)_mm_cmpeq_epi64_mask(__a, __b);
48-
}
49-
50-
__mmask8 test_mm_mask_cmpeq_epi64_mask(__mmask8 __u, __m128i __a, __m128i __b) {
51-
// CHECK-LABEL: @test_mm_mask_cmpeq_epi64_mask
52-
// CHECK: @llvm.x86.avx512.mask.pcmpeq.q.128
53-
return (__mmask8)_mm_mask_cmpeq_epi64_mask(__u, __a, __b);
54-
}
55-
56-
__mmask8 test_mm256_cmpgt_epi32_mask(__m256i __a, __m256i __b) {
57-
// CHECK-LABEL: @test_mm256_cmpgt_epi32_mask
58-
// CHECK: @llvm.x86.avx512.mask.pcmpgt.d.256
59-
return (__mmask8)_mm256_cmpgt_epi32_mask(__a, __b);
60-
}
61-
62-
__mmask8 test_mm256_mask_cmpgt_epi32_mask(__mmask8 __u, __m256i __a, __m256i __b) {
63-
// CHECK-LABEL: @test_mm256_mask_cmpgt_epi32_mask
64-
// CHECK: @llvm.x86.avx512.mask.pcmpgt.d.256
65-
return (__mmask8)_mm256_mask_cmpgt_epi32_mask(__u, __a, __b);
66-
}
67-
68-
__mmask8 test_mm_cmpgt_epi32_mask(__m128i __a, __m128i __b) {
69-
// CHECK-LABEL: @test_mm_cmpgt_epi32_mask
70-
// CHECK: @llvm.x86.avx512.mask.pcmpgt.d.128
71-
return (__mmask8)_mm_cmpgt_epi32_mask(__a, __b);
72-
}
73-
74-
__mmask8 test_mm_mask_cmpgt_epi32_mask(__mmask8 __u, __m128i __a, __m128i __b) {
75-
// CHECK-LABEL: @test_mm_mask_cmpgt_epi32_mask
76-
// CHECK: @llvm.x86.avx512.mask.pcmpgt.d.128
77-
return (__mmask8)_mm_mask_cmpgt_epi32_mask(__u, __a, __b);
78-
}
79-
80-
__mmask8 test_mm256_cmpgt_epi64_mask(__m256i __a, __m256i __b) {
81-
// CHECK-LABEL: @test_mm256_cmpgt_epi64_mask
82-
// CHECK: @llvm.x86.avx512.mask.pcmpgt.q.256
83-
return (__mmask8)_mm256_cmpgt_epi64_mask(__a, __b);
84-
}
85-
86-
__mmask8 test_mm256_mask_cmpgt_epi64_mask(__mmask8 __u, __m256i __a, __m256i __b) {
87-
// CHECK-LABEL: @test_mm256_mask_cmpgt_epi64_mask
88-
// CHECK: @llvm.x86.avx512.mask.pcmpgt.q.256
89-
return (__mmask8)_mm256_mask_cmpgt_epi64_mask(__u, __a, __b);
90-
}
91-
92-
__mmask8 test_mm_cmpgt_epi64_mask(__m128i __a, __m128i __b) {
93-
// CHECK-LABEL: @test_mm_cmpgt_epi64_mask
94-
// CHECK: @llvm.x86.avx512.mask.pcmpgt.q.128
95-
return (__mmask8)_mm_cmpgt_epi64_mask(__a, __b);
96-
}
97-
98-
__mmask8 test_mm_mask_cmpgt_epi64_mask(__mmask8 __u, __m128i __a, __m128i __b) {
99-
// CHECK-LABEL: @test_mm_mask_cmpgt_epi64_mask
100-
// CHECK: @llvm.x86.avx512.mask.pcmpgt.q.128
101-
return (__mmask8)_mm_mask_cmpgt_epi64_mask(__u, __a, __b);
102-
}
103-
1048
__mmask8 test_mm_cmpeq_epu32_mask(__m128i __a, __m128i __b) {
1059
// CHECK-LABEL: @test_mm_cmpeq_epu32_mask
10610
// CHECK: @llvm.x86.avx512.mask.ucmp.d.128(<4 x i32> {{.*}}, <4 x i32> {{.*}}, i32 0, i8 -1)

‎clang/test/CodeGen/target-features-error-2.c

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
// RUN: %clang_cc1 %s -triple=x86_64-linux-gnu -S -verify -o -
2+
#define __MM_MALLOC_H
3+
#include <x86intrin.h>
4+
5+
int baz(__m256i a) {
6+
return _mm256_extract_epi32(a, 3); // expected-error {{function 'baz' and always_inline callee function '_mm256_extract_epi32' are required to have matching target features}}
7+
}

‎clang/test/CodeGen/target-features-error.c

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
// RUN: %clang_cc1 %s -triple=x86_64-linux-gnu -S -verify -o -
2+
int __attribute__((target("avx"), always_inline)) foo(int a) {
3+
return a + 4;
4+
}
5+
int bar() {
6+
return foo(4); // expected-error {{function 'bar' and always_inline callee function 'foo' are required to have matching target features}}
7+
}
8+

0 commit comments

Comments
 (0)