Skip to content

Commit 43a38a6

Browse files
ffrankiesAaronBallman
authored andcommitted
Add a new altera kernel name restriction check to clang-tidy.
The altera kernel name restriction check finds kernel files and include directives whose filename is "kernel.cl", "Verilog.cl", or "VHDL.cl". Such kernel file names cause the Altera Offline Compiler to generate intermediate design files that have the same names as certain internal files, which leads to a compilation error. As per the "Guidelines for Naming the Kernel" section in the "Intel FPGA SDK for OpenCL Pro Edition: Programming Guide."
1 parent 980b860 commit 43a38a6

File tree

25 files changed

+235
-0
lines changed

25 files changed

+235
-0
lines changed

clang-tools-extra/clang-tidy/altera/AlteraTidyModule.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include "../ClangTidy.h"
1010
#include "../ClangTidyModule.h"
1111
#include "../ClangTidyModuleRegistry.h"
12+
#include "KernelNameRestrictionCheck.h"
1213
#include "StructPackAlignCheck.h"
1314

1415
using namespace clang::ast_matchers;
@@ -20,6 +21,8 @@ namespace altera {
2021
class AlteraModule : public ClangTidyModule {
2122
public:
2223
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
24+
CheckFactories.registerCheck<KernelNameRestrictionCheck>(
25+
"altera-kernel-name-restriction");
2326
CheckFactories.registerCheck<StructPackAlignCheck>(
2427
"altera-struct-pack-align");
2528
}

clang-tools-extra/clang-tidy/altera/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ set(LLVM_LINK_COMPONENTS
55

66
add_clang_library(clangTidyAlteraModule
77
AlteraTidyModule.cpp
8+
KernelNameRestrictionCheck.cpp
89
StructPackAlignCheck.cpp
910

1011
LINK_LIBS
Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
//===--- KernelNameRestrictionCheck.cpp - clang-tidy ----------------------===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#include "KernelNameRestrictionCheck.h"
10+
#include "clang/Frontend/CompilerInstance.h"
11+
#include "clang/Lex/PPCallbacks.h"
12+
#include "clang/Lex/Preprocessor.h"
13+
#include <string>
14+
#include <vector>
15+
16+
using namespace clang::ast_matchers;
17+
18+
namespace clang {
19+
namespace tidy {
20+
namespace altera {
21+
22+
namespace {
23+
24+
class KernelNameRestrictionPPCallbacks : public PPCallbacks {
25+
public:
26+
explicit KernelNameRestrictionPPCallbacks(ClangTidyCheck &Check,
27+
const SourceManager &SM)
28+
: Check(Check), SM(SM) {}
29+
30+
void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok,
31+
StringRef FileName, bool IsAngled,
32+
CharSourceRange FileNameRange, const FileEntry *File,
33+
StringRef SearchPath, StringRef RelativePath,
34+
const Module *Imported,
35+
SrcMgr::CharacteristicKind FileType) override;
36+
37+
void EndOfMainFile() override;
38+
39+
private:
40+
/// Returns true if the name of the file with path FilePath is 'kernel.cl',
41+
/// 'verilog.cl', or 'vhdl.cl'. The file name check is case insensitive.
42+
bool FileNameIsRestricted(StringRef FilePath);
43+
44+
struct IncludeDirective {
45+
SourceLocation Loc; // Location in the include directive.
46+
StringRef FileName; // Filename as a string.
47+
};
48+
49+
std::vector<IncludeDirective> IncludeDirectives;
50+
ClangTidyCheck &Check;
51+
const SourceManager &SM;
52+
};
53+
54+
} // namespace
55+
56+
void KernelNameRestrictionCheck::registerPPCallbacks(const SourceManager &SM,
57+
Preprocessor *PP,
58+
Preprocessor *) {
59+
PP->addPPCallbacks(
60+
std::make_unique<KernelNameRestrictionPPCallbacks>(*this, SM));
61+
}
62+
63+
void KernelNameRestrictionPPCallbacks::InclusionDirective(
64+
SourceLocation HashLoc, const Token &, StringRef FileName, bool,
65+
CharSourceRange, const FileEntry *, StringRef, StringRef, const Module *,
66+
SrcMgr::CharacteristicKind) {
67+
IncludeDirective ID = {HashLoc, FileName};
68+
IncludeDirectives.push_back(std::move(ID));
69+
}
70+
71+
bool KernelNameRestrictionPPCallbacks::FileNameIsRestricted(
72+
StringRef FileName) {
73+
return FileName.equals_lower("kernel.cl") ||
74+
FileName.equals_lower("verilog.cl") ||
75+
FileName.equals_lower("vhdl.cl");
76+
}
77+
78+
void KernelNameRestrictionPPCallbacks::EndOfMainFile() {
79+
80+
// Check main file for restricted names.
81+
const FileEntry *Entry = SM.getFileEntryForID(SM.getMainFileID());
82+
StringRef FileName = llvm::sys::path::filename(Entry->getName());
83+
if (FileNameIsRestricted(FileName))
84+
Check.diag(SM.getLocForStartOfFile(SM.getMainFileID()),
85+
"compiling '%0' may cause additional compilation errors due "
86+
"to the name of the kernel source file; consider renaming the "
87+
"included kernel source file")
88+
<< FileName;
89+
90+
if (IncludeDirectives.empty())
91+
return;
92+
93+
// Check included files for restricted names.
94+
for (const IncludeDirective &ID : IncludeDirectives) {
95+
StringRef FileName = llvm::sys::path::filename(ID.FileName);
96+
if (FileNameIsRestricted(FileName))
97+
Check.diag(ID.Loc,
98+
"including '%0' may cause additional compilation errors due "
99+
"to the name of the kernel source file; consider renaming the "
100+
"included kernel source file")
101+
<< FileName;
102+
}
103+
}
104+
105+
} // namespace altera
106+
} // namespace tidy
107+
} // namespace clang
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
//===--- KernelNameRestrictionCheck.h - clang-tidy --------------*- C++ -*-===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ALTERA_KERNEL_NAME_RESTRICTION_CHECK_H
10+
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ALTERA_KERNEL_NAME_RESTRICTION_CHECK_H
11+
12+
#include "../ClangTidyCheck.h"
13+
14+
namespace clang {
15+
namespace tidy {
16+
namespace altera {
17+
18+
/// Finds kernel files and include directives whose filename is `kernel.cl`,
19+
/// `Verilog.cl`, or `VHDL.cl`.
20+
///
21+
/// For the user-facing documentation see:
22+
/// http://clang.llvm.org/extra/clang-tidy/checks/altera-kernel-name-restriction.html
23+
class KernelNameRestrictionCheck : public ClangTidyCheck {
24+
public:
25+
KernelNameRestrictionCheck(StringRef Name, ClangTidyContext *Context)
26+
: ClangTidyCheck(Name, Context) {}
27+
void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
28+
Preprocessor *) override;
29+
};
30+
31+
} // namespace altera
32+
} // namespace tidy
33+
} // namespace clang
34+
35+
#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ALTERA_KERNEL_NAME_RESTRICTION_CHECK_H

clang-tools-extra/docs/ReleaseNotes.rst

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,12 @@ New modules
8585
New checks
8686
^^^^^^^^^^
8787

88+
- New :doc:`altera-kernel-name-restriction
89+
<clang-tidy/checks/kernel-name-restriction>` check.
90+
91+
Finds kernel files and include directives whose filename is `kernel.cl`,
92+
`Verilog.cl`, or `VHDL.cl`.
93+
8894
- New :doc:`altera-struct-pack-align
8995
<clang-tidy/checks/altera-struct-pack-align>` check.
9096

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
.. title:: clang-tidy - altera-kernel-name-restriction
2+
3+
altera-kernel-name-restriction
4+
==============================
5+
6+
Finds kernel files and include directives whose filename is `kernel.cl`,
7+
`Verilog.cl`, or `VHDL.cl`. The check is case insensitive.
8+
9+
Such kernel file names cause the offline compiler to generate intermediate
10+
design files that have the same names as certain internal files, which
11+
leads to a compilation error.
12+
13+
Based on the `Guidelines for Naming the Kernel` section in the
14+
`Intel FPGA SDK for OpenCL Pro Edition: Programming Guide
15+
<https://www.intel.com/content/www/us/en/programmable/documentation/mwh1391807965224.html#ewa1412973930963>`_.

clang-tools-extra/docs/clang-tidy/checks/list.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ Clang-Tidy Checks
3030
`abseil-time-comparison <abseil-time-comparison.html>`_, "Yes"
3131
`abseil-time-subtraction <abseil-time-subtraction.html>`_, "Yes"
3232
`abseil-upgrade-duration-conversions <abseil-upgrade-duration-conversions.html>`_, "Yes"
33+
`altera-kernel-name-restriction <altera-kernel-name-restriction.html>`_,
3334
`altera-struct-pack-align <altera-struct-pack-align.html>`_,
3435
`android-cloexec-accept <android-cloexec-accept.html>`_, "Yes"
3536
`android-cloexec-accept4 <android-cloexec-accept4.html>`_,
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
const int KERNELINT = 1;
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
const int KERNELINT3 = 1;
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
const int OTHERVERILOGINT = 2;
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
const int OTHERDIRVHDLINT = 3;
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
const int OTHERTHINGINT = 1;
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
const int SOMEDIRKERNELINT = 1;
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
int SOME_KERNEL_FOO_INT = 0;
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
int SOME_VERILOG_FOO_INT = 0;
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
int SOME_VHDL_FOO_INT = 0;
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
const int SOMEKERNELINT = 1;
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
const int SOMEDIRVERILOGINT = 2;
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
const int THINGINT = 1;
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
const int VERILOGINT2 = 2;
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
const int VERILOGINT3 = 2;
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
const int VHDLINT2 = 3;
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
const int VHDLINT3 = 3;
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
const int VHDLNUMBERTWOINT = 3;
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
// RUN: %check_clang_tidy %s altera-kernel-name-restriction %t -- -- -I%S/Inputs/altera-kernel-name-restriction
2+
3+
// These are the banned kernel filenames, and should trigger warnings
4+
#include "kernel.cl"
5+
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: including 'kernel.cl' may cause additional compilation errors due to the name of the kernel source file; consider renaming the included kernel source file [altera-kernel-name-restriction]
6+
#include "Verilog.cl"
7+
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: including 'Verilog.cl' may cause additional compilation errors due to the name of the kernel source file; consider renaming the included kernel source file [altera-kernel-name-restriction]
8+
#include "VHDL.cl"
9+
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: including 'VHDL.cl' may cause additional compilation errors due to the name of the kernel source file; consider renaming the included kernel source file [altera-kernel-name-restriction]
10+
11+
// The warning should be triggered regardless of capitalization
12+
#include "KERNEL.cl"
13+
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: including 'KERNEL.cl' may cause additional compilation errors due to the name of the kernel source file; consider renaming the included kernel source file [altera-kernel-name-restriction]
14+
#include "vERILOG.cl"
15+
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: including 'vERILOG.cl' may cause additional compilation errors due to the name of the kernel source file; consider renaming the included kernel source file [altera-kernel-name-restriction]
16+
#include "vhdl.CL"
17+
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: including 'vhdl.CL' may cause additional compilation errors due to the name of the kernel source file; consider renaming the included kernel source file [altera-kernel-name-restriction]
18+
19+
// The warning should be triggered if the names are within a directory
20+
#include "some/dir/kernel.cl"
21+
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: including 'kernel.cl' may cause additional compilation errors due to the name of the kernel source file; consider renaming the included kernel source file [altera-kernel-name-restriction]
22+
#include "somedir/verilog.cl"
23+
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: including 'verilog.cl' may cause additional compilation errors due to the name of the kernel source file; consider renaming the included kernel source file [altera-kernel-name-restriction]
24+
#include "otherdir/vhdl.cl"
25+
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: including 'vhdl.cl' may cause additional compilation errors due to the name of the kernel source file; consider renaming the included kernel source file [altera-kernel-name-restriction]
26+
27+
// There are no FIX-ITs for the altera-kernel-name-restriction lint check
28+
29+
// The following include directives shouldn't trigger the warning
30+
#include "otherthing.cl"
31+
#include "thing.h"
32+
33+
// It doesn't make sense to have kernel.h, verilog.h, or vhdl.h as filenames
34+
// without the corresponding .cl files, but the Altera Programming Guide doesn't
35+
// explicitly forbid it.
36+
#include "kernel.h"
37+
#include "verilog.h"
38+
#include "vhdl.h"
39+
40+
// The files can still have the forbidden names in them, so long as they're not
41+
// the entire file name, and are not the kernel source file name.
42+
#include "some_kernel.cl"
43+
#include "other_Verilog.cl"
44+
#include "vhdl_number_two.cl"
45+
46+
// Naming a directory kernel.cl, verilog.cl, or vhdl.cl is not explicitly
47+
// forbidden in the Altera Programming Guide either.
48+
#include "some/kernel.cl/foo.h"
49+
#include "some/verilog.cl/foo.h"
50+
#include "some/vhdl.cl/foo.h"

0 commit comments

Comments
 (0)