summaryrefslogtreecommitdiffstats
path: root/clang/lib
Commit message (Collapse)AuthorAgeFilesLines
...
* [Sema][NFCI] Don't allocate storage for the various ↵Bruno Ricci2019-03-2522-197/+271
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | CorrectionCandidateCallback unless we are going to do some typo correction The various CorrectionCandidateCallbacks are currently heap-allocated unconditionally. This was needed because of delayed typo correction. However these allocations represent currently 15.4% of all allocations (number of allocations) when parsing all of Boost (!), mostly because of ParseCastExpression, ParseStatementOrDeclarationAfterAttrtibutes and isCXXDeclarationSpecifier. Note that all of these callback objects are small. Let's not do this. Instead initially allocate the callback on the stack, and only do a heap allocation if we are going to do some typo correction. Do this by: 1. Adding a clone function to each callback, which will do a polymorphic clone of the callback. This clone function is required to be implemented by every callback (of which there is a fair amount). Make sure this is the case by making it pure virtual. 2. Use this clone function when we are going to try to correct a typo. This additionally cut the time of -fsyntax-only on all of Boost by 0.5% (not that much, but still something). No functional changes intended. Differential Revision: https://reviews.llvm.org/D58827 Reviewed By: rnk llvm-svn: 356925
* Revert "[clang-format] Keep protobuf "package" statement on one line"Haojian Wu2019-03-251-3/+3
| | | | | | | | | | | | | This reverts commit r356835. This patch causes a regression, see the test below: verifyFormat("// Detached comment\n\n" "// Leading comment\n" "syntax = \"proto2\"; // trailing comment\n\n" "// in foo.bar package\n" "package foo.bar; // foo.bar package\n"); llvm-svn: 356912
* [OpenCL] Allow addr space spelling without __ prefix in C++.Anastasia Stulova2019-03-253-2/+12
| | | | | | | | | | | | | | For backwards compatibility we allow alternative spelling of address spaces - 'private', 'local', 'global', 'constant', 'generic'. In order to accept 'private' correctly, parsing has been changed to understand different use cases - access specifier vs address space. Fixes PR40707 and PR41011! Differential Revision: https://reviews.llvm.org/D59603 llvm-svn: 356888
* [ASTImporter] Changed use of Import to Import_New in ASTNodeImporter.Balazs Keri2019-03-251-50/+22
| | | | | | | | | | | | | | Reviewers: a.sidorin, shafik, martong, a_sidorin Reviewed By: a_sidorin Subscribers: a_sidorin, rnkovacs, gamesh411, dkrupp, martong, Szelethus, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D53757 llvm-svn: 356874
* Un-revert "[coroutines][PR40978] Emit error for co_yield within catch block"Brian Gesiak2019-03-253-17/+54
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | Summary: https://reviews.llvm.org/D59076 added a new coroutine error that prevented users from using 'co_await' or 'co_yield' within a exception handler. However, it was reverted in https://reviews.llvm.org/rC356774 because it caused a regression in nested scopes in C++ catch statements, as documented by https://bugs.llvm.org/show_bug.cgi?id=41171. The issue was due to an incorrect use of a `clang::ParseScope`. To fix: 1. Add a regression test for catch statement parsing that mimics the bug report from https://bugs.llvm.org/show_bug.cgi?id=41171. 2. Re-apply the coroutines error patch from https://reviews.llvm.org/D59076, but this time with the correct ParseScope behavior. Reviewers: GorNishanov, tks2103, rsmith, riccibruno, jbulow Reviewed By: riccibruno Subscribers: EricWF, jdoerfert, lewissbaker, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D59752 llvm-svn: 356865
* [X86] Make _bswap intrinsic a function instead of a macro to hopefully fix ↵Craig Topper2019-03-241-1/+5
| | | | | | | | the chromium build. This intrinsic was added in r356848 but was implemented as a macro to match gcc. llvm-svn: 356862
* [X86] Add BSR/BSF/BSWAP intrinsics to ia32intrin.h to match gcc.Craig Topper2019-03-242-12/+108
| | | | | | | | | | | | | | | | | | | | Summary: These are all implemented by icc as well. I made bit_scan_forward/reverse forward to the __bsfd/__bsrq since we also have __bsfq/__bsrq. Note, when lzcnt is enabled the bsr intrinsics generates lzcnt+xor instead of bsr. Reviewers: RKSimon, spatel Subscribers: cfe-commits, llvm-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D59682 llvm-svn: 356848
* Fix unused variable warning. NFCI.Simon Pilgrim2019-03-231-1/+1
| | | | llvm-svn: 356839
* [clang-format] Keep protobuf "package" statement on one linePaul Hoad2019-03-231-3/+3
| | | | | | | | | | | | | | | | | | | | Summary: Top-level "package" and "import" statements should generally be kept on one line, for all languages. Reviewers: sammccall, krasimir, MyDeveloperDay Reviewed By: MyDeveloperDay Subscribers: MyDeveloperDay, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D59627 Patch By: dchai (Donald Chai) llvm-svn: 356835
* Clang-format: add finer-grained options for putting all arguments on one linePaul Hoad2019-03-232-6/+33
| | | | | | | | | | | | | | | | | | | | | | | | | | Summary: Add two new options, AllowAllArgumentsOnNextLine and AllowAllConstructorInitializersOnNextLine. These mirror the existing AllowAllParametersOfDeclarationOnNextLine and allow me to support an internal style guide where I work. I think this would be generally useful, some have asked for it on stackoverflow: https://stackoverflow.com/questions/30057534/clang-format-binpackarguments-not-working-as-expected https://stackoverflow.com/questions/38635106/clang-format-how-to-prevent-all-function-arguments-on-next-line Reviewers: djasper, krasimir, MyDeveloperDay Reviewed By: MyDeveloperDay Subscribers: jkorous, MyDeveloperDay, aol-nnov, lebedev.ri, uohcsemaj, cfe-commits, klimek Differential Revision: https://reviews.llvm.org/D40988 Patch By: russellmcc (Russell McClellan) llvm-svn: 356834
* [clang-format] correctly format protobuf fields named "enum".Paul Hoad2019-03-231-0/+4
| | | | | | | | | | | | | | | | | | Summary: Similar to TypeScript, "enum" is not a reserved word. Reviewers: krasimir, MyDeveloperDay Reviewed By: MyDeveloperDay Subscribers: MyDeveloperDay, cfe-commits Tags: #clang, #clang-tools-extra Differential Revision: https://reviews.llvm.org/D59629 Patch by: dchai (Donald Chai) llvm-svn: 356833
* IRGen: Remove StructorType; thread GlobalDecl through more code. NFCI.Peter Collingbourne2019-03-2213-229/+113
| | | | | | | | This should make it easier to add more structor variants. Differential Revision: https://reviews.llvm.org/D59724 llvm-svn: 356822
* [WebAssembly] Make driver -pthread imply linker --shared-memoryThomas Lively2019-03-221-1/+3
| | | | | | | | | | | | | | | | Summary: This eliminates a linker error the user might otherwise see about how using the 'atomics' feature requires --shared-memory. Reviewers: sbc100, aheejin Subscribers: dschuff, jgravelle-google, sunfish, jfb, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D59721 llvm-svn: 356817
* [ARM] Fix bug 39982 - pcs("aapcs-vfp") is not consistentCarey Williams2019-03-221-16/+36
| | | | | | | | | | Correctly handle homogeneous aggregates when a function's ABI is specified via the pcs attribute. Bug: https://bugs.llvm.org/show_bug.cgi?id=39982 Differential Revision: https://reviews.llvm.org/D59094 llvm-svn: 356776
* Revert "[coroutines][PR40978] Emit error for co_yield within catch block"Brian Gesiak2019-03-223-57/+19
| | | | | | | | The commit https://reviews.llvm.org/rC356296 is causing a regression in nested catch scopes, https://bugs.llvm.org/show_bug.cgi?id=41171. Revert this change for now in order to un-break that problem report. llvm-svn: 356774
* [OPENMP]Add missing comment, NFC.Alexey Bataev2019-03-221-0/+4
| | | | llvm-svn: 356759
* [OPENMP]Allow no allocator clause in target regions with requiresAlexey Bataev2019-03-221-1/+12
| | | | | | | | | | | | dynamic_allocators. According to the OpenMP 5.0, 2.11.3 allocate Directive, Restrictions, allocate directives that appear in a target region must specify an allocator clause unless a requires directive with the dynamic_allocators clause is present in the same compilation unit. Patch adds a check for a presence of the requires directive with the dynamic_allocators clause. llvm-svn: 356758
* [OPENMP]Emit error message for allocate directive without allocatorAlexey Bataev2019-03-221-1/+5
| | | | | | | | | | | clause in target region. According to the OpenMP 5.0, 2.11.3 allocate Directive, Restrictions, allocate directives that appear in a target region must specify an allocator clause unless a requires directive with the dynamic_allocators clause is present in the same compilation unit. llvm-svn: 356752
* [Tooling] Avoid working-dir races in AllTUsToolExecutorIlya Biryukov2019-03-221-14/+7
| | | | | | | | | | | | | | Reviewers: ioeric Reviewed By: ioeric Subscribers: jdoerfert, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D59683 llvm-svn: 356743
* [AArch64] Split the neon.addp intrinsic into integer and fp variants.Amara Emerson2019-03-211-0/+7
| | | | | | | | | | | | | | | | | | | This is the result of discussions on the list about how to deal with intrinsics which require codegen to disambiguate them via only the integer/fp overloads. It causes problems for GlobalISel as some of that information is lost during translation, while with other operations like IR instructions the information is encoded into the instruction opcode. This patch changes clang to emit the new faddp intrinsic if the vector operands to the builtin have FP element types. LLVM IR AutoUpgrade has been taught to upgrade existing calls to aarch64.neon.addp with fp vector arguments, and we remove the workarounds introduced for GlobalISel in r355865. This is a more permanent solution to PR40968. Differential Revision: https://reviews.llvm.org/D59655 llvm-svn: 356722
* [OPENMP]Fix a warning about unused variable, NFC.Alexey Bataev2019-03-211-0/+1
| | | | llvm-svn: 356715
* Improve the diagnostic for #include_next occurring in a file not foundRichard Smith2019-03-211-0/+4
| | | | | | | | | | | in the include path. Instead of making the incorrect claim that the included file has an absolute path, describe the actual problem: the including file was found either by absolute path, or relative to such a file, or relative to the primary source file. llvm-svn: 356712
* [OPENMP] Simplify codegen for allocate directive on local variables.Alexey Bataev2019-03-212-45/+65
| | | | | | | Simplified codegen for the allocate directive for local variables, initial implementation of the codegen for NVPTX target. llvm-svn: 356710
* [X86] Correct the value of MaxAtomicInlineWidth for pre-586 cpusCraig Topper2019-03-212-5/+18
| | | | | | | | | | Use the new cx8 feature flag that was added to the backend to represent support for cmpxchg8b. Use this flag to set the MaxAtomicInlineWidth. This also assumes all the cmpxchg instructions are enabled for CK_Generic which is what cc1 defaults to when nothing is specified. Differential Revision: https://reviews.llvm.org/D59566 llvm-svn: 356709
* [Driver] Pass -malign-double from the driver to the cc1 command lineCraig Topper2019-03-211-0/+1
| | | | | | | | | | -malign-double is currently only implemented in the -cc1 interface. But its declared in Options.td so it is a driver option too. But you try to use it with the driver you'll get a message about the option being unused. This patch teaches the driver to pass the option through to cc1 so it won't be unused. The Options.td says the option is x86 only but I didn't see any x86 specific code in its impementation in cc1 so not sure if the documentation is wrong or if I should only pass this option through the driver on x86 targets. Differential Revision: https://reviews.llvm.org/D59624 llvm-svn: 356706
* [CodeGen][ObjC] Annotate calls to objc_retainAutoreleasedReturnValueAkira Hatanaka2019-03-213-11/+28
| | | | | | | | | | | | | with notail on x86-64. On x86-64, the epilogue code inserted before the tail jump blocks the autoreleased return optimization. rdar://problem/38675807 Differential Revision: https://reviews.llvm.org/D59656 llvm-svn: 356705
* Refactor handling of #include directives to cleanly separate theRichard Smith2019-03-211-33/+49
| | | | | | | "skipped header because it should be imported as a module" cases from the "skipped header because of some other reason" cases. llvm-svn: 356704
* [OPENMP]Codegen support for allocate directive on global variables.Alexey Bataev2019-03-215-0/+67
| | | | | | | | | For the global variables the allocate directive must specify only the predefined allocator. This allocator must be translated into the correct form of the address space for the targets that support different address spaces. llvm-svn: 356702
* [OPENMP]Simplify the check for the predefined allocators, NFC.Alexey Bataev2019-03-211-39/+7
| | | | | | | | Previously implemented check required the reevaluation of the already evaluated predefined allocator kind for the global variables. Patch simplifies this evaluation and removes extra code. llvm-svn: 356699
* [X86] Add __popcntd and __popcntq to ia32intrin.h to match gcc and icc. ↵Craig Topper2019-03-212-32/+42
| | | | | | | | | | Remove popcnt feature flag from _popcnt32/_popcnt64 and move to ia32intrin.h to match gcc gcc and icc both implement popcntd and popcntq which we did not. gcc doesn't seem to require a feature flag for the _popcnt32/_popcnt64 spelling and will use a libcall if its not supported. Differential Revision: https://reviews.llvm.org/D59567 llvm-svn: 356689
* [X86] Use the CPUKind enum from PROC_ALIAS to directly get the CPUKind in ↵Craig Topper2019-03-211-3/+2
| | | | | | | | fillValidCPUList. We were using getCPUKind which translates the string to the enum also using PROC_ALIAS. This just cuts out the string compares. llvm-svn: 356686
* [ASTMatchers][OpenMP] OpenMP Structured-block-related matchersRoman Lebedev2019-03-211-0/+3
| | | | | | | | | | | | | | | | | Summary: Exposes to the for ASTMatchers the interface/modelling of OpenMP structured-block. Reviewers: gribozavr, aaron.ballman, JonasToth, george.karpenkov Reviewed By: gribozavr, aaron.ballman Subscribers: guansong, jdoerfert, cfe-commits Tags: #clang, #openmp Differential Revision: https://reviews.llvm.org/D59463 llvm-svn: 356676
* [ASTTypeTraits][ASTMatchers][OpenMP] OMPClause handlingRoman Lebedev2019-03-214-0/+49
| | | | | | | | | | | | | | | | | | | Summary: `OMPClause` is the base class, it is not descendant from **any** other class, therefore for it to work with e.g. `VariadicDynCastAllOfMatcher<>`, it needs to be handled here. Reviewers: sbenza, bkramer, pcc, klimek, hokein, gribozavr, aaron.ballman, george.karpenkov Reviewed By: gribozavr, aaron.ballman Subscribers: guansong, jdoerfert, alexfh, ABataev, cfe-commits Tags: #openmp, #clang Differential Revision: https://reviews.llvm.org/D57112 llvm-svn: 356675
* [ASTMatchers][OpenMP] Add base ompExecutableDirective() matcher.Roman Lebedev2019-03-212-0/+4
| | | | | | | | | | | | | | | | | | Summary: A simple matcher for `OMPExecutableDirective` Stmt type. Split off from D57113. Reviewers: gribozavr, aaron.ballman, JonasToth, george.karpenkov Reviewed By: gribozavr, aaron.ballman Subscribers: guansong, jdoerfert, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D59453 llvm-svn: 356674
* Permit redeclarations of a builtin to specify calling convention.Erich Keane2019-03-213-13/+21
| | | | | | | | | | | | | | | | | | | | After https://reviews.llvm.org/rL355317 we noticed that quite a decent amount of code redeclares builtins (memcpy in particular, I believe reduced from an MSVC header) with a calling convention specified. This gets particularly troublesome when the user specifies a new 'default' calling convention on the command line. When looking to add a diagnostic for this case, it was noticed that we had 3 other diagnostics that differed only slightly. This patch ALSO unifies those under a 'select'. Unfortunately, the order of words in ONE of these diagnostics was reversed ("'thiscall' calling convention" vs "calling convention 'thiscall'"), so this patch also standardizes on the former. Differential Revision: https://reviews.llvm.org/D59560 Change-Id: I79f99fe7c2301640755ffdd774b46eb44526bb22 llvm-svn: 356663
* [clang-format] Add basic support for formatting C# filesPaul Hoad2019-03-218-13/+352
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Summary: This revision adds basic support for formatting C# files with clang-format, I know the barrier to entry is high here so I'm sending this revision in to test the water as to whether this might be something we'd consider landing. Tracking in Bugzilla as: https://bugs.llvm.org/show_bug.cgi?id=40850 Justification: C# code just looks ugly in comparison to the C++ code in our source tree which is clang-formatted. I've struggled with Visual Studio reformatting to get a clean and consistent style, I want to format our C# code on saving like I do now for C++ and i want it to have the same style as defined in our .clang-format file, so it consistent as it can be with C++. (Braces/Breaking/Spaces/Indent etc..) Using clang format without this patch leaves the code in a bad state, sometimes when the BreakStringLiterals is set, it fails to compile. Mostly the C# is similar to Java, except instead of JavaAnnotations I try to reuse the TT_AttributeSquare. Almost the most valuable portion is to have a new Language in order to partition the configuration for C# within a common .clang-format file, with the auto detection on the .cs extension. But there are other C# specific styles that could be added later if this is accepted. in particular how `{ set;get }` is formatted. Reviewers: djasper, klimek, krasimir, benhamilton, JonasToth Reviewed By: klimek Subscribers: llvm-commits, mgorny, jdoerfert, cfe-commits Tags: #clang, #clang-tools-extra Differential Revision: https://reviews.llvm.org/D58404 llvm-svn: 356662
* [CFG] [analyzer] pr41142: C++17: Skip transparent InitListExprs in constructors.Artem Dergachev2019-03-211-0/+9
| | | | | | | | | | | | | | When searching for construction contexts, i.e. figuring out which statements define the object that is constructed by each construct-expression, ignore transparent init-list expressions because they don't add anything to the context. This allows the Static Analyzer to model construction, destruction, materialization, lifetime extension correctly in more cases. Also fixes a crash caused by incorrectly evaluating initial values of variables initialized with such expressions. Differential Revision: https://reviews.llvm.org/D59573 llvm-svn: 356634
* Revert "[clangd] Print arguments in template specializations"Jordan Rupprecht2019-03-201-16/+1
| | | | | | This reverts commit 44a63f6a150dec72dea43730d2a89d292e58bd6f. It segfaults on an internal test case (will follow up off thread). llvm-svn: 356623
* [clang-format] BeforeHash added to IndentPPDirectivesPaul Hoad2019-03-204-10/+20
| | | | | | | | | | | | | | | | | | Summary: The option BeforeHash added to IndentPPDirectives. Fixes Bug 36019. https://bugs.llvm.org/show_bug.cgi?id=36019 Reviewers: djasper, klimek, krasimir, sammccall, mprobst, Nicola, MyDeveloperDay Reviewed By: klimek, MyDeveloperDay Subscribers: kadircet, MyDeveloperDay, mnussbaum, geleji, ufna, cfe-commits Patch by to-mix. Differential Revision: https://reviews.llvm.org/D52150 llvm-svn: 356613
* [X86] Add __crc32b/__crc32w/__crc32d/__crc32q intrinsics to match gcc and icc.Craig Topper2019-03-201-0/+86
| | | | | | | | | gcc has these intrinsics in ia32intrin.h as well. And icc implements them though they aren't documented in the Intel Intrinsics Guide. Differential Revision: https://reviews.llvm.org/D59533 llvm-svn: 356609
* [OPENMP]Improve detection of omp_allocator_handle_t type and predefinedAlexey Bataev2019-03-203-32/+117
| | | | | | | | | | | allocators. It is better to deduce omp_allocator_handle_t type from the predefined allocators, because omp.h header might not define it explicitly. Plus, it allows to identify the predefined allocators correctly when trying to build the allcoator for the global variables. llvm-svn: 356607
* Fix implicit ios -> watchOS availability version mapping forAlex Lorenz2019-03-201-0/+1
| | | | | | | | versions that have the major number only rdar://48018651 llvm-svn: 356605
* [Sema] Deduplicate some availability checking logicErik Pilkington2019-03-204-67/+15
| | | | | | | | | | | | | | | Before this commit, we emit unavailable errors for calls to functions during overload resolution, and for references to all other declarations in DiagnoseUseOfDecl. The early checks during overload resolution aren't as good as the DiagnoseAvailabilityOfDecl based checks, as they error on the code from PR40991. This commit fixes this by removing the early checking. llvm.org/PR40991 rdar://48564179 Differential revision: https://reviews.llvm.org/D59394 llvm-svn: 356599
* Recommit "Support attribute used in member funcs of class templates"Rafael Auler2019-03-201-0/+14
| | | | | | | | | | | | | | | | | | | | This diff previously exposed a bug in LLVM's IRLinker, breaking buildbots that tried to self-host LLVM with monolithic LTO. The bug is now in LLVM by D59552 Original commit message: As PR17480 describes, clang does not support the used attribute for member functions of class templates. This means that if the member function is not used, its definition is never instantiated. This patch changes clang to emit the definition if it has the used attribute. Test Plan: Added a testcase Reviewed By: aaron.ballman Differential Revision: https://reviews.llvm.org/D56928 llvm-svn: 356598
* Remove the unused return value in ASTImporter::Imported [NFC]Raphael Isemann2019-03-201-2/+1
| | | | | | | | | | | | | | | | | | Summary: `ASTImporter::Imported` currently returns a Decl, but that return value is not used by the ASTImporter (or anywhere else) nor is it documented. Reviewers: balazske, martong, a.sidorin, shafik Reviewed By: balazske, martong Subscribers: rnkovacs, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D59595 llvm-svn: 356592
* [X86] Remove getCPUKindCanonicalName which is unused.Craig Topper2019-03-202-14/+0
| | | | | | Differential Revision: https://reviews.llvm.org/D59578 llvm-svn: 356580
* [NFC][ASTMatchers] Alphabetically sort REGISTER_MATCHER() macros in ↵Roman Lebedev2019-03-201-16/+17
| | | | | | | | RegistryMaps::RegistryMaps() As noted in https://reviews.llvm.org/D59453#inline-526253 llvm-svn: 356578
* [clang-format] structured binding in range for detected as Objective CPaul Hoad2019-03-201-1/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Summary: Sometime after 6.0.0 and the current trunk 9.0.0 the following code would be considered as objective C and not C++ Reported by: https://twitter.com/mattgodbolt/status/1096188576503644160 $ clang-format.exe test.h Configuration file(s) do(es) not support Objective-C: C:\clang\build\.clang-format --- test.h -- ``` std::vector<std::pair<std::string,std::string>> C; void foo() { for (auto && [A,B] : C) { std::string D = A + B; } } ``` The following code fixes this issue of incorrect detection Reviewers: djasper, klimek, JonasToth, reuk Reviewed By: klimek Subscribers: cfe-commits Tags: #clang-tools-extra Differential Revision: https://reviews.llvm.org/D59546 llvm-svn: 356575
* [OpenCL] Generate 'unroll.enable' metadata for ↵Andrew Savonichev2019-03-201-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | __attribute__((opencl_unroll_hint)) Summary: [OpenCL] Generate 'unroll.enable' metadata for __attribute__((opencl_unroll_hint)) For both !{!"llvm.loop.unroll.enable"} and !{!"llvm.loop.unroll.full"} the unroller will try to fully unroll a loop unless the trip count is not known at compile time. In that case for '.full' metadata no unrolling will be processed, while for '.enable' the loop will be partially unrolled with a heuristically chosen unroll factor. See: docs/LanguageExtensions.rst From https://www.khronos.org/registry/OpenCL/sdk/2.0/docs/man/xhtml/attributes-loopUnroll.html __attribute__((opencl_unroll_hint)) for (int i=0; i<2; i++) { ... } In the example above, the compiler will determine how much to unroll the loop. Before the patch for __attribute__((opencl_unroll_hint)) was generated metadata !{!"llvm.loop.unroll.full"}, which limits ability of loop unroller to decide, how much to unroll the loop. Reviewers: Anastasia, yaxunl Reviewed By: Anastasia Subscribers: zzheng, dmgreen, jdoerfert, cfe-commits, asavonic, AlexeySotkin Tags: #clang Differential Revision: https://reviews.llvm.org/D59493 llvm-svn: 356571
* [clang][OpeMP] Model OpenMP structured-block in AST (PR40563)Roman Lebedev2019-03-206-4/+47
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Summary: https://www.openmp.org/wp-content/uploads/OpenMP-API-Specification-5.0.pdf, page 3: ``` structured block For C/C++, an executable statement, possibly compound, with a single entry at the top and a single exit at the bottom, or an OpenMP construct. COMMENT: See Section 2.1 on page 38 for restrictions on structured blocks. ``` ``` 2.1 Directive Format Some executable directives include a structured block. A structured block: • may contain infinite loops where the point of exit is never reached; • may halt due to an IEEE exception; • may contain calls to exit(), _Exit(), quick_exit(), abort() or functions with a _Noreturn specifier (in C) or a noreturn attribute (in C/C++); • may be an expression statement, iteration statement, selection statement, or try block, provided that the corresponding compound statement obtained by enclosing it in { and } would be a structured block; and Restrictions Restrictions to structured blocks are as follows: • Entry to a structured block must not be the result of a branch. • The point of exit cannot be a branch out of the structured block. C / C++ • The point of entry to a structured block must not be a call to setjmp(). • longjmp() and throw() must not violate the entry/exit criteria. ``` Of particular note here is the fact that OpenMP structured blocks are as-if `noexcept`, in the same sense as with the normal `noexcept` functions in C++. I.e. if throw happens, and it attempts to travel out of the `noexcept` function (here: out of the current structured-block), then the program terminates. Now, one of course can say that since it is explicitly prohibited by the Specification, then any and all programs that violate this Specification contain undefined behavior, and are unspecified, and thus no one should care about them. Just don't write broken code /s But i'm not sure this is a reasonable approach. I have personally had oss-fuzz issues of this origin - exception thrown inside of an OpenMP structured-block that is not caught, thus causing program termination. This issue isn't all that hard to catch, it's not any particularly different from diagnosing the same situation with the normal `noexcept` function. Now, clang static analyzer does not presently model exceptions. But clang-tidy has a simplisic [[ https://clang.llvm.org/extra/clang-tidy/checks/bugprone-exception-escape.html | bugprone-exception-escape ]] check, and it is even refactored as a `ExceptionAnalyzer` class for reuse. So it would be trivial to use that analyzer to check for exceptions escaping out of OpenMP structured blocks. (D59466) All that sounds too great to be true. Indeed, there is a caveat. Presently, it's practically impossible to do. To check a OpenMP structured block you need to somehow 'get' the OpenMP structured block, and you can't because it's simply not modelled in AST. `CapturedStmt`/`CapturedDecl` is not it's representation. Now, it is of course possible to write e.g. some AST matcher that would e.g. match every OpenMP executable directive, and then return the whatever `Stmt` is the structured block of said executable directive, if any. But i said //practically//. This isn't practical for the following reasons: 1. This **will** bitrot. That matcher will need to be kept up-to-date, and refreshed with every new OpenMP spec version. 2. Every single piece of code that would want that knowledge would need to have such matcher. Well, okay, if it is an AST matcher, it could be shared. But then you still have `RecursiveASTVisitor` and friends. `2 > 1`, so now you have code duplication. So it would be reasonable (and is fully within clang AST spirit) to not force every single consumer to do that work, but instead store that knowledge in the correct, and appropriate place - AST, class structure. Now, there is another hoop we need to get through. It isn't fully obvious //how// to model this. The best solution would of course be to simply add a `OMPStructuredBlock` transparent node. It would be optimal, it would give us two properties: * Given this `OMPExecutableDirective`, what's it OpenMP structured block? * It is trivial to check whether the `Stmt*` is a OpenMP structured block (`isa<OMPStructuredBlock>(ptr)`) But OpenMP structured block isn't **necessarily** the first, direct child of `OMP*Directive`. (even ignoring the clang's `CapturedStmt`/`CapturedDecl` that were inserted inbetween). So i'm not sure whether or not we could re-create AST statements after they were already created? There would be other costs to a new AST node: https://bugs.llvm.org/show_bug.cgi?id=40563#c12 ``` 1. You will need to break the representation of loops. The body should be replaced by the "structured block" entity. 2. You will need to support serialization/deserialization. 3. You will need to support template instantiation. 4. You will need to support codegen and take this new construct to account in each OpenMP directive. ``` Instead, there **is** an functionally-equivalent, alternative solution, consisting of two parts. Part 1: * Add a member function `isStandaloneDirective()` to the `OMPExecutableDirective` class, that will tell whether this directive is stand-alone or not, as per the spec. We need it because we can't just check for the existance of associated statements, see code comment. * Add a member function `getStructuredBlock()` to the OMPExecutableDirective` class itself, that assert that this is not a stand-alone directive, and either return the correct loop body if this is a loop-like directive, or the captured statement. This way, given an `OMPExecutableDirective`, we can get it's structured block. Also, since the knowledge is ingrained into the clang OpenMP implementation, it will not cause any duplication, and //hopefully// won't bitrot. Great we achieved 1 of 2 properties of `OMPStructuredBlock` approach. Thus, there is a second part needed: * How can we check whether a given `Stmt*` is `OMPStructuredBlock`? Well, we can't really, in general. I can see this workaround: ``` class FunctionASTVisitor : public RecursiveASTVisitor<FunctionASTVisitor> { using Base = RecursiveASTVisitor<FunctionASTVisitor>; public: bool VisitOMPExecDir(OMPExecDir *D) { OmpStructuredStmts.emplace_back(D.getStructuredStmt()); } bool VisitSOMETHINGELSE(???) { if(InOmpStructuredStmt) HI! } bool TraverseStmt(Stmt *Node) { if (!Node) return Base::TraverseStmt(Node); if (OmpStructuredStmts.back() == Node) ++InOmpStructuredStmt; Base::TraverseStmt(Node); if (OmpStructuredStmts.back() == Node) { OmpStructuredStmts.pop_back(); --InOmpStructuredStmt; } return true; } std::vector<Stmt*> OmpStructuredStmts; int InOmpStructuredStmt = 0; }; ``` But i really don't see using it in practice. It's just too intrusive; and again, requires knowledge duplication. .. but no. The solution lies right on the ground. Why don't we simply store this `i'm a openmp structured block` in the bitfield of the `Stmt` itself? This does not appear to have any impact on the memory footprint of the clang AST, since it's just a single extra bit in the bitfield. At least the static assertions don't fail. Thus, indeed, we can achieve both of the properties without a new AST node. We can cheaply set that bit right in sema, at the end of `Sema::ActOnOpenMPExecutableDirective()`, by just calling the `getStructuredBlock()` that we just added. Test coverage that demonstrates all this has been added. This isn't as great with serialization though. Most of it does not use abbrevs, so we do end up paying the full price (4 bytes?) instead of a single bit. That price, of course, can be reclaimed by using abbrevs. In fact, i suspect that //might// not just reclaim these bytes, but pack these PCH significantly. I'm not seeing a third solution. If there is one, it would be interesting to hear about it. ("just don't write code that would require `isa<OMPStructuredBlock>(ptr)`" is not a solution.) Fixes [[ https://bugs.llvm.org/show_bug.cgi?id=40563 | PR40563 ]]. Reviewers: ABataev, rjmccall, hfinkel, rsmith, riccibruno, gribozavr Reviewed By: ABataev, gribozavr Subscribers: mgorny, aaron.ballman, steveire, guansong, jfb, jdoerfert, cfe-commits Tags: #clang, #openmp Differential Revision: https://reviews.llvm.org/D59214 llvm-svn: 356570
OpenPOWER on IntegriCloud