[PATCH] D66564: [clang-tidy] FPGA struct pack align lint check

ffrankies updated this revision to Diff 216516. ffrankies added a comment. Noticed that some of the lint checks were missing from docs/clang-tidy/checks/list.rst Added the fpga-struct-pack-align lint check to the update checks list. CHANGES SINCE LAST ACTION

[PATCH] D66564: [clang-tidy] FPGA struct pack align lint check

ffrankies created this revision. ffrankies added a reviewer: alexfh. ffrankies created this object with visibility "All Users". ffrankies added a project: clang-tools-extra. Herald added subscribers: arphaman, xazax.hun, Anastasia, mgorny, srhines. Herald added a reviewer: jdoerfert. Herald added

[PATCH] D66564: [clang-tidy] new FPGA struct pack align check

ffrankies updated this revision to Diff 217224. ffrankies added a comment. Added space after clang-tidy in header comments, updated check documentation to use link syntax. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66564/new/ https://reviews.llvm.org/D66564 Files:

[PATCH] D66564: [clang-tidy] new FPGA struct pack align check

ffrankies updated this revision to Diff 216928. ffrankies added a comment. Implemented changes requested by Eugene.Zelenko CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66564/new/ https://reviews.llvm.org/D66564 Files: clang-tidy/fpga/CMakeLists.txt

[PATCH] D66564: [clang-tidy] new FPGA struct pack align check

ffrankies marked 22 inline comments as done. ffrankies added a comment. In D66564#1647779 , @BlackAngel35 wrote: > > Please mention new module and check in Release Notes. The new module and check are now mentioned in Release Notes. P.S. The above

[PATCH] D66564: [clang-tidy] new FPGA struct pack align check

ffrankies updated this revision to Diff 219358. ffrankies added a comment. Herald added subscribers: kadircet, jkorous. Sorry for the delay. I was mostly having trouble building clang-tidy after pulling from master and having it recognize that the struct-pack-align check exists. I finally

[PATCH] D66564: [clang-tidy] new FPGA struct pack align check

ffrankies added a subscriber: alexandre.isoard. ffrankies added a comment. Herald added a subscriber: usaxena95. @alexandre.isoard wrote: > I'm not sure what is the advantage of this compared to -Wpadded? This option only warns when padding exists. Our check does two things; it warns when

[PATCH] D70094: [clang-tidy] new altera ID dependent backward branch check

ffrankies updated this revision to Diff 228743. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70094/new/ https://reviews.llvm.org/D70094 Files: clang-tidy/CMakeLists.txt clang-tidy/ClangTidyForceLinker.h clang-tidy/altera/AlteraTidyModule.cpp clang-tidy/altera/CMakeLists.txt

[PATCH] D70094: [clang-tidy] new altera ID dependent backward branch check

ffrankies created this revision. ffrankies added reviewers: alexfh, jdoerfert, hokein, aaron.ballman. ffrankies added projects: clang-tools-extra, clang, LLVM. Herald added subscribers: mgehre, arphaman, xazax.hun, Anastasia, mgorny. This lint check is a part of the FLOCL (FPGA Linters for

[PATCH] D70094: [clang-tidy] new altera ID dependent backward branch check

ffrankies updated this revision to Diff 232724. ffrankies marked 2 inline comments as done. ffrankies added a comment. Addressed @Eugene.Zelenko's comments on use of `auto` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70094/new/ https://reviews.llvm.org/D70094 Files:

[PATCH] D66564: [clang-tidy] new altera struct pack align check

ffrankies updated this revision to Diff 230828. ffrankies edited the summary of this revision. ffrankies added a comment. Implemented changes requested by @aaron.ballman - Moved the check from the "performance" module into the new "altera" module - Split the diagnostic message into a warning and

[PATCH] D70094: [clang-tidy] new altera ID dependent backward branch check

ffrankies updated this revision to Diff 231309. ffrankies added a comment. Implemented changes requested by @Eugene.Zelenko Also changed for loop in `hasIdDepVar` and `hasIdDepField` to range-based for loops, and updated the license information in IdDependentBackwardBranchCheck.cpp CHANGES

[PATCH] D66564: [clang-tidy] new performance struct pack align check

ffrankies added a comment. As per the previous discussion, the check has been moved into the `performance` module. In D66564#1670640 , @lebedev.ri wrote: > I, too, don't believe this is FPGA specific; it should likely go into `misc-` > or even

[PATCH] D66564: [clang-tidy] new performance struct pack align check

ffrankies updated this revision to Diff 224799. ffrankies retitled this revision from "[clang-tidy] new FPGA struct pack align check" to "[clang-tidy] new performance struct pack align check". ffrankies edited the summary of this revision. CHANGES SINCE LAST ACTION

[PATCH] D66564: [clang-tidy] new performance struct pack align check

ffrankies updated this revision to Diff 224802. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66564/new/ https://reviews.llvm.org/D66564 Files: clang-tidy/performance/CMakeLists.txt clang-tidy/performance/PerformanceTidyModule.cpp clang-tidy/performance/StructPackAlignCheck.cpp

[PATCH] D66564: [clang-tidy] new performance struct pack align check

ffrankies updated this revision to Diff 224803. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66564/new/ https://reviews.llvm.org/D66564 Files: clang-tidy/performance/CMakeLists.txt clang-tidy/performance/PerformanceTidyModule.cpp clang-tidy/performance/StructPackAlignCheck.cpp

[PATCH] D72241: [clang-tidy] new altera single work item barrier check

ffrankies updated this revision to Diff 243717. ffrankies marked 3 inline comments as done. ffrankies added a comment. Implemented requested changes by @Eugene.Zelenko - Changed `auto` to `const auto *` - Changed `if(IsNDRange == true)` to `if(IsNDRange)` - Highlighted 1600 with single

[PATCH] D72235: [clang-tidy] new altera unroll loops check

ffrankies updated this revision to Diff 248322. ffrankies added a comment. @Eugene.Zelenko It turns out we were using an old mirror of the clang-tools-extra repository, that no longer seems to be updated. I switched to the llvm-project

[PATCH] D66564: [clang-tidy] new altera struct pack align check

ffrankies updated this revision to Diff 246871. ffrankies marked 5 inline comments as done. ffrankies added a comment. Addressed comments by @aaron.ballman - Removed commented-out code and irrelevant FIXMEs - Added Fix-Its to insert/amend the aligned and packed struct attributes as needed.

[PATCH] D72235: [clang-tidy] new altera unroll loops check

ffrankies updated this revision to Diff 246993. ffrankies marked 9 inline comments as done. ffrankies added a comment. Addressed comments by @Eugene.Zelenko - Used const auto * when type is mentioned in same sentence - Fixed formatting in documentation - Documented default value of

[PATCH] D72218: [clang-tidy] new altera kernel name restriction check

ffrankies updated this revision to Diff 243945. ffrankies marked 9 inline comments as done. ffrankies edited the summary of this revision. ffrankies added a comment. Implemented changes requested by @Eugene.Zelenko: - Added empty lines around namespace block - Fixed use of auto keyword - Fixed

[PATCH] D72239: [clang-tidy] new opencl recursion not supported check

ffrankies added a comment. In D72239#1815583 , @JonasToth wrote: > The other recursion check seems more sophisticated. What is your take on it? > Would you consider it better as well, what is your opinion on it? I agree, I would go with the other check

[PATCH] D72218: [clang-tidy] new altera kernel name restriction check

ffrankies created this revision. ffrankies added reviewers: alexfh, hokein, aaron.ballman. ffrankies added projects: clang-tools-extra, clang, LLVM. Herald added subscribers: mgehre, ormris, arphaman, xazax.hun, Anastasia, mgorny. This lint check is part of the FLOCL (FPGA Linters for OpenCL)

[PATCH] D72239: [clang-tidy] new opencl recursion not supported check

ffrankies created this revision. ffrankies added reviewers: aaron.ballman, alexfh, hokein. ffrankies added projects: clang, clang-tools-extra, LLVM. Herald added subscribers: mgehre, arphaman, xazax.hun, Anastasia, yaxunl, mgorny. This lint check is part of the FLOCL (FPGA Linters for OpenCL)

[PATCH] D72235: [clang-tidy] new altera unroll loops check

ffrankies updated this revision to Diff 236270. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72235/new/ https://reviews.llvm.org/D72235 Files: clang-tidy/CMakeLists.txt clang-tidy/ClangTidyForceLinker.h clang-tidy/altera/AlteraTidyModule.cpp clang-tidy/altera/CMakeLists.txt

[PATCH] D72235: [clang-tidy] new altera unroll loops check

ffrankies created this revision. ffrankies added reviewers: aaron.ballman, hokein, alexfh. ffrankies added projects: clang-tools-extra, clang, LLVM. Herald added subscribers: mgehre, arphaman, zzheng, xazax.hun, Anastasia, mgorny. ffrankies updated this revision to Diff 236270. ffrankies updated

[PATCH] D72235: [clang-tidy] new altera unroll loops check

ffrankies updated this revision to Diff 236271. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72235/new/ https://reviews.llvm.org/D72235 Files: clang-tidy/CMakeLists.txt clang-tidy/ClangTidyForceLinker.h clang-tidy/altera/AlteraTidyModule.cpp clang-tidy/altera/CMakeLists.txt

[PATCH] D72241: [clang-tidy] new altera single work item barrier check

ffrankies created this revision. ffrankies added reviewers: aaron.ballman, alexfh, hokein, Eugene.Zelenko. ffrankies added projects: clang-tools-extra, clang. Herald added subscribers: mgehre, jfb, arphaman, xazax.hun, Anastasia, mgorny. This lint check is a part of the FLOCL (FPGA Linters for

[PATCH] D66564: [clang-tidy] new altera struct pack align check

ffrankies updated this revision to Diff 254277. ffrankies added a comment. Addressed comments by @aaron.ballman You're right, we don't want this to trigger on template instantiations and structs declared in system headers. - Check no longer triggers on template instantiations - Check no longer

[PATCH] D72218: [clang-tidy] new altera kernel name restriction check

ffrankies updated this revision to Diff 254301. ffrankies added a comment. - Updated underlying repo to https://github.com/llvm/llvm-project - Removed braces from one-line if-statements CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72218/new/ https://reviews.llvm.org/D72218 Files:

[PATCH] D72241: [clang-tidy] new altera single work item barrier check

ffrankies updated this revision to Diff 254304. ffrankies added a comment. - Updated underlying repo to https://github.com/llvm/llvm-project - Removed braces from one-line if-statements CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72241/new/ https://reviews.llvm.org/D72241 Files:

[PATCH] D70094: [clang-tidy] new altera ID dependent backward branch check

ffrankies updated this revision to Diff 254305. ffrankies marked 5 inline comments as done. ffrankies added a comment. - Updated underlying repo to https://github.com/llvm/llvm-project - Removed braces from one-line if-statements CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70094/new/

[PATCH] D72235: [clang-tidy] new altera unroll loops check

ffrankies updated this revision to Diff 254299. ffrankies added a comment. Addressed comments by @Eugene.Zelenko - Removed braces from one-lien if statements - Release notes on the altera unroll loops check now match the first line of documentation. CHANGES SINCE LAST ACTION

[PATCH] D66564: [clang-tidy] new altera struct pack align check

ffrankies added a comment. @Eugene.Zelenko Just checking in, is there anything I missed regarding what we need to do for these checks? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66564/new/ https://reviews.llvm.org/D66564

[PATCH] D66564: [clang-tidy] new altera struct pack align check

ffrankies added a comment. In D66564#2257635 , @aaron.ballman wrote: > In D66564#2256482 , @ffrankies wrote: > >> In D66564#2256424 , @Eugene.Zelenko >> wrote: >> >>> In

[PATCH] D66564: [clang-tidy] new altera struct pack align check

ffrankies updated this revision to Diff 290396. ffrankies added a comment. Rebased, changed import in `StructPackAlignCheck.h` from `../ClangTidy.h` to `../ClangTidyCheck.h` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66564/new/ https://reviews.llvm.org/D66564 Files:

[PATCH] D66564: [clang-tidy] new altera struct pack align check

ffrankies added a comment. In D66564#2260836 , @aaron.ballman wrote: > I've committed on your behalf in 156b127945a8c923d141e608b7380427da024376 > . Thank > you for the new check! No

[PATCH] D66564: [clang-tidy] new altera struct pack align check

ffrankies added a comment. In D66564#2256424 , @Eugene.Zelenko wrote: > In D66564#2256423 , @ffrankies wrote: > >> @Eugene.Zelenko I don't have commit access to the repository, could you >> please commit this

[PATCH] D66564: [clang-tidy] new altera struct pack align check

ffrankies added a comment. @Eugene.Zelenko I don't have commit access to the repository, could you please commit this check on our behalf? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66564/new/ https://reviews.llvm.org/D66564 ___

[PATCH] D72218: [clang-tidy] new altera kernel name restriction check

ffrankies updated this revision to Diff 298601. ffrankies marked 7 inline comments as done. ffrankies added a comment. Addressed comments by @aaron.ballman regarding the diagnostic warning. I tried to add a test case for when the filename is `kernel.cl`, `verilog.cl`, or `vhdl.cl`, but that did

[PATCH] D72218: [clang-tidy] new altera kernel name restriction check

ffrankies updated this revision to Diff 294215. ffrankies edited the summary of this revision. ffrankies added a comment. Herald added a project: LLVM. Herald added a subscriber: llvm-commits. - Rebased code and fixed merge conflicts with D66564 - Added

[PATCH] D72241: [clang-tidy] new altera single work item barrier check

ffrankies updated this revision to Diff 295698. ffrankies removed a project: clang. ffrankies added a comment. Herald added a project: LLVM. Herald added a subscriber: llvm-commits. - Rebased code and fixed merge conflicts with D66564 - Added

[PATCH] D72218: [clang-tidy] new altera kernel name restriction check

ffrankies updated this revision to Diff 295697. ffrankies removed a project: LLVM. ffrankies added a comment. Herald added a project: LLVM. Addressed changes requested by @Eugene.Zelenko and @aaron.ballman CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72218/new/

[PATCH] D66564: [clang-tidy] new altera struct pack align check

ffrankies updated this revision to Diff 283106. ffrankies added a comment. Clarified a comment: we don't want the warnings to trigger on templated struct //declarations//, not instantiations. We don't want it to trigger on any instantiations. Added a test case that checks if warnings are

[PATCH] D66564: [clang-tidy] new altera struct pack align check

ffrankies added a comment. @njames93 Thanks for the clarification! Your suggestion worked, but then I realized that I was working off of an improperly worded comment, which I've corrected. I looked through the AST Matcher reference, and didn't find anything that would trigger on a templated

[PATCH] D66564: [clang-tidy] new altera struct pack align check

ffrankies added a comment. Posted this as an inline comment, copying here: I tried to move this into the matcher like this: Finder->addMatcher(recordDecl(isStruct(), isDefinition(), unless(isExpansionInSystemHeader()),

[PATCH] D72241: [clang-tidy] new altera single work item barrier check

ffrankies updated this revision to Diff 309565. ffrankies added a comment. Implemented changes requested by @aaron.ballman - using `hasAnyName()` instead of multiple `hasName()` calls in the matcher - switched to a combination of `hasAttr<>()` and `getAttr<>()` to remove need for casting and

[PATCH] D72241: [clang-tidy] new altera single work item barrier check

ffrankies updated this revision to Diff 312658. ffrankies marked an inline comment as done. ffrankies added a comment. @aaron.ballman hmm, that is strange. I've rebased the patch and updated the diff, let me know if this one doesn't work either or there's something else you'd like me to try.

[PATCH] D72235: [clang-tidy] new altera unroll loops check

ffrankies updated this revision to Diff 312675. ffrankies marked 9 inline comments as done. ffrankies added a comment. - Rebased with master branch - Diagnostics are re-worded so they're not complete sentences Check ready for further review CHANGES SINCE LAST ACTION

[PATCH] D72235: [clang-tidy] new altera unroll loops check

ffrankies updated this revision to Diff 312676. ffrankies added a comment. - removed unnecessary comment CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72235/new/ https://reviews.llvm.org/D72235 Files: clang-tools-extra/clang-tidy/altera/AlteraTidyModule.cpp

[PATCH] D72235: [clang-tidy] new altera unroll loops check

ffrankies updated this revision to Diff 313174. ffrankies added a comment. - Addressed comments from @njames93 - Rebased with latest master branch to make sure there are no merge issues with the latest committed altera check CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72235/new/

[PATCH] D72241: [clang-tidy] new altera single work item barrier check

ffrankies added a comment. @aaron.ballman @njames93 thanks for testing this out, the feedback, and the commit! Could you also take a look at D72235 unroll loops check ? It's been updated and is ready for review. CHANGES SINCE LAST ACTION

[PATCH] D72241: [clang-tidy] new altera single work item barrier check

ffrankies marked 4 inline comments as done. ffrankies added a comment. @aaron.ballman Thank you! If there are no further comments, could you please commit this on my behalf? My GitHub username is ffrankies . Comment at:

[PATCH] D72218: [clang-tidy] new altera kernel name restriction check

ffrankies updated this revision to Diff 303680. ffrankies added a comment. Moved test files `KERNEL.cl`, `VHDL.cl` and `vERILOG.cl` to the `uppercase` subdirectory to prevent filename clashes in some environments. This is in response to the buildbot failure where `Verilog.cl`, `KERNEL.cl`, and

[PATCH] D72218: [clang-tidy] new altera kernel name restriction check

ffrankies marked 3 inline comments as done. ffrankies added a comment. @aaron.ballman Can you please commit this on my behalf? My github username is ffrankies . And could you take a look at D72241 altera single work item barrier check

[PATCH] D72218: [clang-tidy] new altera kernel name restriction check

ffrankies updated this revision to Diff 301865. ffrankies marked an inline comment as done. ffrankies added a comment. Implemented changes requested by @aaron.ballman - Added a helper function that implements the string comparison logic - Clarified that check is case insensitive - Removed unused

[PATCH] D70094: [clang-tidy] new altera ID dependent backward branch check

ffrankies marked 2 inline comments as done. ffrankies added a comment. @aaron.ballman Thank you! As far as I'm aware, this is the last check that we are planning to submit, so if I do get commit access now it's likely to be unused. However, if that does change, then yes I would be interested in

[PATCH] D70094: [clang-tidy] new altera ID dependent backward branch check

ffrankies marked an inline comment as done. ffrankies added a comment. @Eugene.Zelenko @aaron.ballman Are there any more changes that need to be made to this check or comments that need to be addressed? Comment at:

[PATCH] D70094: [clang-tidy] new altera ID dependent backward branch check

ffrankies updated this revision to Diff 342278. ffrankies added a comment. Addressed comments by @aaron.ballman - Changed capitalization in enum - Used `std::move` in `IdDependencyRecord` constructors - Initialized `VariableDeclaration` and `FieldDeclaration` to `nullptr` - Used

[PATCH] D70094: [clang-tidy] new altera ID dependent backward branch check

ffrankies updated this revision to Diff 342280. ffrankies marked 12 inline comments as done. ffrankies added a comment. Removed unused import statements from IdDependentBackwardBranch.cpp CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70094/new/ https://reviews.llvm.org/D70094 Files:

[PATCH] D70094: [clang-tidy] new altera ID dependent backward branch check

ffrankies updated this revision to Diff 342467. ffrankies added a comment. Addressed comment by @aaron.ballman and the Pre-update checks linter - Removed calls to `std::move` for `llvm::Twine::str()` object in `IdDependencyRecord` constructors CHANGES SINCE LAST ACTION

[PATCH] D70094: [clang-tidy] new altera ID dependent backward branch check

ffrankies updated this revision to Diff 335195. ffrankies added a comment. - Addressed comments from the automated pre-merge checks - Moved link to external documentation to the end of `clang-tools-extra/docs/clang-tidy/checks/altera-id-dependent-backward-branch.rst` as requested by

[PATCH] D70094: [clang-tidy] new altera ID dependent backward branch check

ffrankies updated this revision to Diff 334979. ffrankies added a comment. - Rebased on top of latest changes in main branch - The diagnostic that identifies the code location where an ID-dependent variable/field is assigned has been changed from a warning to a note - Changes addressing code

[PATCH] D72235: [clang-tidy] new altera unroll loops check

ffrankies added a comment. Thank you very much! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72235/new/ https://reviews.llvm.org/D72235 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D72235: [clang-tidy] new altera unroll loops check

ffrankies updated this revision to Diff 332303. ffrankies added a comment. Implemented changes requested by @aaron.ballman CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72235/new/ https://reviews.llvm.org/D72235 Files: clang-tools-extra/clang-tidy/altera/AlteraTidyModule.cpp

[PATCH] D72235: [clang-tidy] new altera unroll loops check

ffrankies marked 7 inline comments as done. ffrankies added a comment. @aaron.ballman If there are no more changes, can you please commit this on my behalf? My github username is ffrankies . CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72235/new/

[PATCH] D72235: [clang-tidy] new altera unroll loops check

ffrankies updated this revision to Diff 328935. ffrankies marked 4 inline comments as done. ffrankies added a comment. - Removed unnecessary braces. - Simplified boolean return statements. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72235/new/ https://reviews.llvm.org/D72235 Files:

[PATCH] D72235: [clang-tidy] new altera unroll loops check

ffrankies updated this revision to Diff 328933. ffrankies added a comment. - Surrounded language constructs with double back ticks in `clang-tools-extra/docs/clang-tidy/checks/altera-unroll-loops.rst`. - Removed trailing whitespace. - Added single quotes around `#pragma unroll`s in diagnostics.

[PATCH] D72235: [clang-tidy] new altera unroll loops check

ffrankies updated this revision to Diff 329082. ffrankies marked 6 inline comments as done. ffrankies added a comment. Rebased with the latest main branch to fix patch application errors. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72235/new/ https://reviews.llvm.org/D72235 Files:

[PATCH] D72235: [clang-tidy] new altera unroll loops check

ffrankies updated this revision to Diff 329207. ffrankies added a comment. - Ran `git-clang-format HEAD^` - Addressed comments automatically added by the Lint: Pre-merge Checks CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72235/new/ https://reviews.llvm.org/D72235 Files:

[PATCH] D72235: [clang-tidy] new altera unroll loops check

ffrankies updated this revision to Diff 328579. ffrankies marked 12 inline comments as done. ffrankies added a comment. - Added support for `CXXForRangeStmt` loops - Added support for different for loop increments (`++`, `--`, `+=`, `-=`, `*=`, `\=`) - Depending on the exit condition, the

[PATCH] D70094: [clang-tidy] new altera ID dependent backward branch check

ffrankies updated this revision to Diff 338160. ffrankies marked 3 inline comments as done. ffrankies added a comment. Removed `*- C++ -*` from IdDependentBackwardBranchCheck.cpp file-level comment. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70094/new/

[PATCH] D70094: [clang-tidy] new altera ID dependent backward branch check

ffrankies updated this revision to Diff 336512. ffrankies added a comment. Addressed comments by @Eugene.Zelenko and the automated - Fixed header comments and include guard style - Removed unnecessary comments in `getLoopType()` - changed `IDDependencyRecord() {}` to `IDDependencyRecord() =