[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
ebevhan added inline comments. Comment at: lib/Basic/TargetInfo.cpp:45 + AccumWidth = AccumAlign = 32; + LongAccumWidth = LongAccumAlign = 64; SuitableAlign = 64; rsmith wrote: > jfb wrote: > > This seems weird because Targets which don't have these values for the > > non-Accum versions will have .e.g. `sizeof(short) != sizeof(short _Accum)`. > > Is there a point in ever having `_Accum` differ in size, width, and > > alignment from the underlying type? If not, can you set these values after > > the sub-target has specified its preferences? > I'm uncomfortable about opting all targets into this behavior with these > default values; this will result in an ABI break if later a target updates > these to the correct values. A per-target `AccumSupported` flag would help > substantially. But this is OK for the short term while you're still working > on the feature. > > We'll also need the target to inform us of the number of integer and > fractional bits for each such type. > We'll also need the target to inform us of the number of integer and > fractional bits for each such type. I believe the only one that is needed is for the number of fractional bits; the number of integer bits is implied by the difference between the type width and fractional bits. I think I mention this in one of the other patches. Repository: rC Clang https://reviews.llvm.org/D46084 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47445: [ASTImporter] Corrected diagnostic client handling in tests.
balazske added a comment. If `BeginSourceFile` is not called on the diagnostic client object, it is not possible to have compiler warnings or errors that come from the "To" context while importing something (there is some assertion if a source file related warning is to be emitted but no source file is set). In the currently existing tests there is no such case but I have not yet committed tests that produce such warnings (for example ODR violations during import or encounter of unsupported constructs). Repository: rC Clang https://reviews.llvm.org/D47445 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46911: [Fixed Point Arithmetic] Addition of the remaining fixed point types and their saturated equivalents
ebevhan added inline comments. Comment at: include/clang/AST/Type.h:6551 + +QualType getCorrespondingSaturatedType(const ASTContext &Context, + const QualType &Ty); These should probably be in ASTContext directly. Comment at: include/clang/Basic/TargetInfo.h:83 + unsigned char LongFractWidth, LongFractAlign; + unsigned char SatShortAccumWidth, SatShortAccumAlign; + unsigned char SatAccumWidth, SatAccumAlign; I don't think the saturating types need separate configurations. Embedded-C says "Each saturating fixed-point type has the same representation and the same rank as its corresponding primary fixed-point type." Comment at: lib/Parse/ParseDecl.cpp:3614 + } else { +isInvalid = DS.SetTypeSpecSat(/*isSat=*/true, Loc, PrevSpec, DiagID); + } Is there a use for the isSat parameter? Comment at: lib/Sema/DeclSpec.cpp:1123 +if (!(TypeSpecType == TST_accum || TypeSpecType == TST_fract)) { + S.Diag(TSSatLoc, diag::err_invalid_saturation_spec) + << getSpecifierName((TST)TypeSpecType, Policy); Handling this case here means that placing _Sat on something other than exactly a fixed-point type is a parsing error rather than a semantic error. How does this handle _Sat on sugared types? Should _Sat on things like typedefs work? typedef _Fract myfract; _Sat myfract F; The primary issue (and this is one that we have encountered as well) is that you cannot have a true _Sat typedef since _Sat only exists as part of builtin types. You need to desugar/canonicalize the type and then do getCorrespondingSaturatingType (or have getCorrespondingSaturatingType look at the canonical type internally). Comment at: lib/Sema/DeclSpec.cpp:1135 TypeSpecType != TST_char && TypeSpecType != TST_wchar && - TypeSpecType != TST_accum) { + TypeSpecType != TST_accum && TypeSpecType != TST_fract) { S.Diag(TSSLoc, diag::err_invalid_sign_spec) IsFixedPointType can be used here as well. Comment at: lib/Sema/DeclSpec.cpp:1165 else if (TypeSpecType != TST_int && TypeSpecType != TST_double && - TypeSpecType != TST_accum) { + TypeSpecType != TST_accum && TypeSpecType != TST_fract) { S.Diag(TSWRange.getBegin(), diag::err_invalid_width_spec) IsFixedPointType? Comment at: lib/Sema/SemaType.cpp:1410 + +if (DS.getTypeSpecSign() != DeclSpec::TSS_unsigned) + Result = fixedpoint::getCorrespondingSignedType(Context, Result); The logic is a bit reversed. The default should be to select the signed variant, and if the TSS is unsigned, then convert it to the unsigned variant. Comment at: lib/Sema/SemaType.cpp:1609 declarator.setInvalidType(true); // Handle complex types. Other qualifiers like const and volatile are handled down here. Should the _Sat application be performed somewhere here instead? Repository: rC Clang https://reviews.llvm.org/D46911 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47445: [ASTImporter] Corrected diagnostic client handling in tests.
balazske added a comment. The new `ASTUnit::beginSourceFile` is only there to simplify the code. It is possible to get `Ctx`, `PP` and `getDiagnostic()` from outside of `ASTUnit` and call the same thing, but requires more code to write. Probably a more smart place to call `BeginSourceFile` can be found if we look deep into `ASTUnit` and `buildAST` functions that generate the AST from code that has associated source file but this would be a change that affects more code. Repository: rC Clang https://reviews.llvm.org/D47445 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47460: Treat files as volatile by default
yvvan updated this revision to Diff 149249. yvvan added a comment. This is the proper fix. When we get a buffer for main file we should use the UserFilesAreVolatile flag to specify if memory mapping needs to occur or not. https://reviews.llvm.org/D47460 Files: lib/Frontend/ASTUnit.cpp Index: lib/Frontend/ASTUnit.cpp === --- lib/Frontend/ASTUnit.cpp +++ lib/Frontend/ASTUnit.cpp @@ -100,7 +100,8 @@ static std::unique_ptr getBufferForFileHandlingRemapping(const CompilerInvocation &Invocation, vfs::FileSystem *VFS, - StringRef FilePath) { + StringRef FilePath, + bool isVolatile) { const auto &PreprocessorOpts = Invocation.getPreprocessorOpts(); // Try to determine if the main file has been remapped, either from the @@ -120,7 +121,8 @@ llvm::sys::fs::UniqueID MID = MPathStatus->getUniqueID(); if (MainFileID == MID) { // We found a remapping. Try to load the resulting, remapped source. - BufferOwner = valueOrNull(VFS->getBufferForFile(RF.second)); + BufferOwner = valueOrNull( + VFS->getBufferForFile(RF.second, -1, true, isVolatile)); if (!BufferOwner) return nullptr; } @@ -145,7 +147,8 @@ // If the main source file was not remapped, load it now. if (!Buffer && !BufferOwner) { -BufferOwner = valueOrNull(VFS->getBufferForFile(FilePath)); +BufferOwner = +valueOrNull(VFS->getBufferForFile(FilePath, -1, true, isVolatile)); if (!BufferOwner) return nullptr; } @@ -1231,7 +1234,7 @@ PreambleInvocationIn.getFrontendOpts().Inputs[0].getFile(); std::unique_ptr MainFileBuffer = getBufferForFileHandlingRemapping(PreambleInvocationIn, VFS.get(), -MainFilePath); +MainFilePath, UserFilesAreVolatile); if (!MainFileBuffer) return nullptr; Index: lib/Frontend/ASTUnit.cpp === --- lib/Frontend/ASTUnit.cpp +++ lib/Frontend/ASTUnit.cpp @@ -100,7 +100,8 @@ static std::unique_ptr getBufferForFileHandlingRemapping(const CompilerInvocation &Invocation, vfs::FileSystem *VFS, - StringRef FilePath) { + StringRef FilePath, + bool isVolatile) { const auto &PreprocessorOpts = Invocation.getPreprocessorOpts(); // Try to determine if the main file has been remapped, either from the @@ -120,7 +121,8 @@ llvm::sys::fs::UniqueID MID = MPathStatus->getUniqueID(); if (MainFileID == MID) { // We found a remapping. Try to load the resulting, remapped source. - BufferOwner = valueOrNull(VFS->getBufferForFile(RF.second)); + BufferOwner = valueOrNull( + VFS->getBufferForFile(RF.second, -1, true, isVolatile)); if (!BufferOwner) return nullptr; } @@ -145,7 +147,8 @@ // If the main source file was not remapped, load it now. if (!Buffer && !BufferOwner) { -BufferOwner = valueOrNull(VFS->getBufferForFile(FilePath)); +BufferOwner = +valueOrNull(VFS->getBufferForFile(FilePath, -1, true, isVolatile)); if (!BufferOwner) return nullptr; } @@ -1231,7 +1234,7 @@ PreambleInvocationIn.getFrontendOpts().Inputs[0].getFile(); std::unique_ptr MainFileBuffer = getBufferForFileHandlingRemapping(PreambleInvocationIn, VFS.get(), -MainFilePath); +MainFilePath, UserFilesAreVolatile); if (!MainFileBuffer) return nullptr; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46190: For a referenced declaration, mark any associated usings as referenced.
CarlosAlbertoEnciso added a comment. Ping. Thanks https://reviews.llvm.org/D46190 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r333637 - [Driver] Clean up tmp files when deleting Compilation objects
Author: dstenb Date: Thu May 31 02:05:22 2018 New Revision: 333637 URL: http://llvm.org/viewvc/llvm-project?rev=333637&view=rev Log: [Driver] Clean up tmp files when deleting Compilation objects Summary: In rL327851 the createUniqueFile() and createTemporaryFile() variants that do not return the file descriptors were changed to create empty files, rather than only check if the paths are free. This change was done in order to make the functions race-free. That change led to clang-tidy (and possibly other tools) leaving behind temporary assembly files, of the form placeholder-*, when using a target that does not support the internal assembler. The temporary files are created when building the Compilation object in stripPositionalArgs(), as a part of creating the compilation database for the arguments after the double-dash. The files are created by Driver::GetNamedOutputPath(). Fix this issue by cleaning out temporary files at the deletion of Compilation objects. This fixes https://bugs.llvm.org/show_bug.cgi?id=37091. Reviewers: klimek, sepavloff, arphaman, aaron.ballman, john.brawn, mehdi_amini, sammccall, bkramer, alexfh, JDevlieghere Reviewed By: aaron.ballman, JDevlieghere Subscribers: erichkeane, lebedev.ri, Ka-Ka, cfe-commits Differential Revision: https://reviews.llvm.org/D45686 Modified: cfe/trunk/include/clang/Driver/Compilation.h cfe/trunk/lib/Driver/Compilation.cpp cfe/trunk/lib/Driver/Driver.cpp Modified: cfe/trunk/include/clang/Driver/Compilation.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Compilation.h?rev=333637&r1=333636&r2=333637&view=diff == --- cfe/trunk/include/clang/Driver/Compilation.h (original) +++ cfe/trunk/include/clang/Driver/Compilation.h Thu May 31 02:05:22 2018 @@ -122,6 +122,9 @@ class Compilation { /// Whether an error during the parsing of the input args. bool ContainsError; + /// Whether to keep temporary files regardless of -save-temps. + bool ForceKeepTempFiles = false; + public: Compilation(const Driver &D, const ToolChain &DefaultToolChain, llvm::opt::InputArgList *Args, Modified: cfe/trunk/lib/Driver/Compilation.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Compilation.cpp?rev=333637&r1=333636&r2=333637&view=diff == --- cfe/trunk/lib/Driver/Compilation.cpp (original) +++ cfe/trunk/lib/Driver/Compilation.cpp Thu May 31 02:05:22 2018 @@ -45,6 +45,11 @@ Compilation::Compilation(const Driver &D } Compilation::~Compilation() { + // Remove temporary files. This must be done before arguments are freed, as + // the file names might be derived from the input arguments. + if (!TheDriver.isSaveTempsEnabled() && !ForceKeepTempFiles) +CleanupFileList(TempFiles); + delete TranslatedArgs; delete Args; @@ -245,6 +250,10 @@ void Compilation::initCompilationForDiag AllActions.clear(); Jobs.clear(); + // Remove temporary files. + if (!TheDriver.isSaveTempsEnabled() && !ForceKeepTempFiles) +CleanupFileList(TempFiles); + // Clear temporary/results file lists. TempFiles.clear(); ResultFiles.clear(); @@ -262,6 +271,9 @@ void Compilation::initCompilationForDiag // Redirect stdout/stderr to /dev/null. Redirects = {None, {""}, {""}}; + + // Temporary files added by diagnostics should be kept. + ForceKeepTempFiles = true; } StringRef Compilation::getSysRoot() const { Modified: cfe/trunk/lib/Driver/Driver.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=333637&r1=333636&r2=333637&view=diff == --- cfe/trunk/lib/Driver/Driver.cpp (original) +++ cfe/trunk/lib/Driver/Driver.cpp Thu May 31 02:05:22 2018 @@ -1253,9 +1253,6 @@ void Driver::generateCompilationDiagnost // If any of the preprocessing commands failed, clean up and exit. if (!FailingCommands.empty()) { -if (!isSaveTempsEnabled()) - C.CleanupFileList(C.getTempFiles(), true); - Diag(clang::diag::note_drv_command_failed_diag_msg) << "Error generating preprocessed source(s)."; return; @@ -1372,9 +1369,6 @@ int Driver::ExecuteCompilation( C.ExecuteJobs(C.getJobs(), FailingCommands); - // Remove temp files. - C.CleanupFileList(C.getTempFiles()); - // If the command succeeded, we are done. if (FailingCommands.empty()) return 0; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r333637 - [Driver] Clean up tmp files when deleting Compilation objects
Author: dstenb Date: Thu May 31 02:05:22 2018 New Revision: 333637 URL: http://llvm.org/viewvc/llvm-project?rev=333637&view=rev Log: [Driver] Clean up tmp files when deleting Compilation objects Summary: In rL327851 the createUniqueFile() and createTemporaryFile() variants that do not return the file descriptors were changed to create empty files, rather than only check if the paths are free. This change was done in order to make the functions race-free. That change led to clang-tidy (and possibly other tools) leaving behind temporary assembly files, of the form placeholder-*, when using a target that does not support the internal assembler. The temporary files are created when building the Compilation object in stripPositionalArgs(), as a part of creating the compilation database for the arguments after the double-dash. The files are created by Driver::GetNamedOutputPath(). Fix this issue by cleaning out temporary files at the deletion of Compilation objects. This fixes https://bugs.llvm.org/show_bug.cgi?id=37091. Reviewers: klimek, sepavloff, arphaman, aaron.ballman, john.brawn, mehdi_amini, sammccall, bkramer, alexfh, JDevlieghere Reviewed By: aaron.ballman, JDevlieghere Subscribers: erichkeane, lebedev.ri, Ka-Ka, cfe-commits Differential Revision: https://reviews.llvm.org/D45686 Added: clang-tools-extra/trunk/test/clang-tidy/pr37091.cpp Added: clang-tools-extra/trunk/test/clang-tidy/pr37091.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/pr37091.cpp?rev=333637&view=auto == --- clang-tools-extra/trunk/test/clang-tidy/pr37091.cpp (added) +++ clang-tools-extra/trunk/test/clang-tidy/pr37091.cpp Thu May 31 02:05:22 2018 @@ -0,0 +1,10 @@ +// REQUIRES: shell +// RUN: rm -rf %t +// RUN: mkdir -p %t + +// This is a reproducer for PR37091. +// +// Verify that no temporary files are left behind by the clang-tidy invocation. + +// RUN: env TMPDIR=%t TEMP=%t TMP=%t clang-tidy %s -- --target=mips64 +// RUN: rmdir %t ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46013: [ARM] Conform to AAPCS when passing overaligned composites as arguments
chill updated this revision to Diff 149252. chill added a comment. Update: - similar changes needed for AArch64 - added/updated tests https://reviews.llvm.org/D46013 Files: include/clang/AST/ASTContext.h include/clang/AST/RecordLayout.h lib/AST/ASTContext.cpp lib/AST/RecordLayout.cpp lib/AST/RecordLayoutBuilder.cpp lib/CodeGen/TargetInfo.cpp test/CodeGen/aapcs-align.cc test/CodeGen/aapcs64-align.cc test/CodeGen/arm-arguments.c test/CodeGen/arm64-arguments.c Index: test/CodeGen/arm64-arguments.c === --- test/CodeGen/arm64-arguments.c +++ test/CodeGen/arm64-arguments.c @@ -216,11 +216,11 @@ typedef __attribute__((neon_vector_type(4))) int int32x4_t; int32x4_t f36(int i, s36_with_align s1, s36_with_align s2) { -// CHECK: define <4 x i32> @f36(i32 %i, i128 %s1.coerce, i128 %s2.coerce) +// CHECK: define <4 x i32> @f36(i32 %i, [2 x i64] %s1.coerce, [2 x i64] %s2.coerce) // CHECK: %s1 = alloca %struct.s36, align 16 // CHECK: %s2 = alloca %struct.s36, align 16 -// CHECK: store i128 %s1.coerce, i128* %{{.*}}, align 16 -// CHECK: store i128 %s2.coerce, i128* %{{.*}}, align 16 +// CHECK: store [2 x i64] %s1.coerce, [2 x i64]* %{{.*}}, align 16 +// CHECK: store [2 x i64] %s2.coerce, [2 x i64]* %{{.*}}, align 16 // CHECK: %[[a:.*]] = bitcast %struct.s36* %s1 to <4 x i32>* // CHECK: load <4 x i32>, <4 x i32>* %[[a]], align 16 // CHECK: %[[b:.*]] = bitcast %struct.s36* %s2 to <4 x i32>* @@ -325,11 +325,11 @@ // passing aligned structs in registers __attribute__ ((noinline)) int f39(int i, s39_with_align s1, s39_with_align s2) { -// CHECK: define i32 @f39(i32 %i, i128 %s1.coerce, i128 %s2.coerce) +// CHECK: define i32 @f39(i32 %i, [2 x i64] %s1.coerce, [2 x i64] %s2.coerce) // CHECK: %s1 = alloca %struct.s39, align 16 // CHECK: %s2 = alloca %struct.s39, align 16 -// CHECK: store i128 %s1.coerce, i128* %{{.*}}, align 16 -// CHECK: store i128 %s2.coerce, i128* %{{.*}}, align 16 +// CHECK: store [2 x i64] %s1.coerce, [2 x i64]* %{{.*}}, align 16 +// CHECK: store [2 x i64] %s2.coerce, [2 x i64]* %{{.*}}, align 16 // CHECK: getelementptr inbounds %struct.s39, %struct.s39* %s1, i32 0, i32 0 // CHECK: getelementptr inbounds %struct.s39, %struct.s39* %s2, i32 0, i32 0 // CHECK: getelementptr inbounds %struct.s39, %struct.s39* %s1, i32 0, i32 1 @@ -340,31 +340,31 @@ s39_with_align g39_2; int caller39() { // CHECK: define i32 @caller39() -// CHECK: %[[a:.*]] = load i128, i128* bitcast (%struct.s39* @g39 to i128*), align 16 -// CHECK: %[[b:.*]] = load i128, i128* bitcast (%struct.s39* @g39_2 to i128*), align 16 -// CHECK: call i32 @f39(i32 3, i128 %[[a]], i128 %[[b]]) +// CHECK: %[[a:.*]] = load [2 x i64], [2 x i64]* bitcast (%struct.s39* @g39 to [2 x i64]*), align 16 +// CHECK: %[[b:.*]] = load [2 x i64], [2 x i64]* bitcast (%struct.s39* @g39_2 to [2 x i64]*), align 16 +// CHECK: call i32 @f39(i32 3, [2 x i64] %[[a]], [2 x i64] %[[b]]) return f39(3, g39, g39_2); } // passing aligned structs on stack __attribute__ ((noinline)) int f39_stack(int i, int i2, int i3, int i4, int i5, int i6, int i7, int i8, int i9, s39_with_align s1, s39_with_align s2) { -// CHECK: define i32 @f39_stack(i32 %i, i32 %i2, i32 %i3, i32 %i4, i32 %i5, i32 %i6, i32 %i7, i32 %i8, i32 %i9, i128 %s1.coerce, i128 %s2.coerce) +// CHECK define i32 @f39_stack(i32 %i, i32 %i2, i32 %i3, i32 %i4, i32 %i5, i32 %i6, i32 %i7, i32 %i8, i32 %i9, [2 x i64] %s1.coerce, [2 x i64] %s2.coerce) // CHECK: %s1 = alloca %struct.s39, align 16 // CHECK: %s2 = alloca %struct.s39, align 16 -// CHECK: store i128 %s1.coerce, i128* %{{.*}}, align 16 -// CHECK: store i128 %s2.coerce, i128* %{{.*}}, align 16 +// CHECK: store [2 x i64] %s1.coerce, [2 x i64]* %{{.*}}, align 16 +// CHECK: store [2 x i64] %s2.coerce, [2 x i64]* %{{.*}}, align 16 // CHECK: getelementptr inbounds %struct.s39, %struct.s39* %s1, i32 0, i32 0 // CHECK: getelementptr inbounds %struct.s39, %struct.s39* %s2, i32 0, i32 0 // CHECK: getelementptr inbounds %struct.s39, %struct.s39* %s1, i32 0, i32 1 // CHECK: getelementptr inbounds %struct.s39, %struct.s39* %s2, i32 0, i32 1 return s1.i + s2.i + i + i2 + i3 + i4 + i5 + i6 + i7 + i8 + i9 + s1.s + s2.s; } int caller39_stack() { // CHECK: define i32 @caller39_stack() -// CHECK: %[[a:.*]] = load i128, i128* bitcast (%struct.s39* @g39 to i128*), align 16 -// CHECK: %[[b:.*]] = load i128, i128* bitcast (%struct.s39* @g39_2 to i128*), align 16 -// CHECK: call i32 @f39_stack(i32 1, i32 2, i32 3, i32 4, i32 5, i32 6, i32 7, i32 8, i32 9, i128 %[[a]], i128 %[[b]]) +// CHECK: %[[a:.*]] = load [2 x i64], [2 x i64]* bitcast (%struct.s39* @g39 to [2 x i64]*), align 16 +// CHECK: %[[b:.*]] = load [2 x i64], [2 x i64]* bitcast (%struct.s39* @g39_2 to [2 x i64]*), align 16 +// CHECK: call i32 @f39_stack(i32 1, i32 2, i32 3, i32 4, i32 5, i32 6, i32 7, i32 8, i32 9, [2 x i64] %[[a]], [2 x i64] %[[b]]) return f39_stack(1, 2, 3, 4, 5, 6, 7, 8, 9, g
[PATCH] D47532: [ASTImporter] Import the whole redecl chain of functions
martong added a comment. I just wanted to give a detailed justification about why we should import the whole redecl chain. Consider the following code: void f(); // prototype void f() { f(); } Currently, when we import the prototype we end up having two independent functions with definitions: TranslationUnitDecl 0x25214c8 <> | |-FunctionDecl 0x255e3e8 col:16 f 'void ()' | `-CompoundStmt 0x255e4f0 | `-CallExpr 0x255e4c8 'void' | `-ImplicitCastExpr 0x255e4b0 'void (*)()' | `-DeclRefExpr 0x255e488 'void ()' lvalue Function 0x255e3e8 'f' 'void ()' `-FunctionDecl 0x255e300 col:6 f 'void ()' `-CompoundStmt 0x255e570 `-CallExpr 0x255e548 'void' `-ImplicitCastExpr 0x255e530 'void (*)()' `-DeclRefExpr 0x255e508 'void ()' lvalue Function 0x255e3e8 'f' 'void ()' Also, when we import a definition of the friend function below we again end up with two different definitions: struct X { friend void f(); }; void f(){} And what's more, one of these definitions has the class as a parent: `-CXXRecordDecl 0x1802358 col:8 struct X definition |-CXXRecordDecl 0x1802478 col:8 implicit struct X |-FunctionDecl 0x1802560 parent 0x17c5618 col:24 f 'void ()' | `-CompoundStmt 0x1802610 `-FriendDecl 0x1802620 col:24 `-FunctionDecl 0x1802560 parent 0x17c5618 col:24 f 'void ()' `-CompoundStmt 0x1802610 In previous versions, we had similar problems in case of virtual functions, i.e. the virtual flag of an out-of-class definition was not set, because we missed to import the prototype which had the explicit `virtual` keyword. That seems to be fixed even before this patch, but we wouldn't have that problem if we had import the whole redecl chain. These redundant definitions can cause assertions and can mislead any tool which tries to process such a malformed AST. Repository: rC Clang https://reviews.llvm.org/D47532 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47577: [clang-format] Separate block comments with CRLF correctly
Uran198 created this revision. Uran198 added a reviewer: alexfh. Herald added subscribers: cfe-commits, klimek. When formatting the following string: "/*\r\n" " * Comment with\r\n" "\r\n" " * blanks.\r\n" " */\r\n" clang-format produced: "/*\r\n" " * Comment with\r\n" "\r\r\n" " * blanks.\r\n" " */\r\n" And when formatting "#define A(\\\r\n" "x) /* \\\r\n" "a comment \\\r\n" "inside */ \\\r\n" " f();" with line length 17, clang-format produced: "#define A( \\\r" "x) /* \\ \\\r" "a comment \\ \\\r" "inside */ \\\r" " f();" So in one case it added additional `\r` instead of replacing with the blank line and in another it added additional newline escape character `\`. After the change the result are respectively: "/*\r\n" " * Comment with\r\n" "\r\n" " * blanks .\r\ n" " */\r\n" and "#define A(x) /* \\\r\n" " a comment \\\r\n" " inside */ \\\r\n" " f();" Repository: rC Clang https://reviews.llvm.org/D47577 Files: lib/Format/BreakableToken.cpp unittests/Format/FormatTestComments.cpp Index: unittests/Format/FormatTestComments.cpp === --- unittests/Format/FormatTestComments.cpp +++ unittests/Format/FormatTestComments.cpp @@ -472,6 +472,30 @@ " int jjj; /*b*/"); } +TEST_F(FormatTestComments, BlockCommentsWithCLRF) { + EXPECT_EQ("/*\r\n" +" * Comment with\r\n" +"\r\n" +" * blanks.\r\n" +" */\r\n" +"void f() {}", +format("/* \r\n" +" * Comment with\r\n" +" \r\n" +" * blanks.\r\n" +" */\r\n" +"void f() {}")); + EXPECT_EQ("#define A(x) /* \\\r\n" +" a comment \\\r\n" +" inside */ \\\r\n" +" f();", +format("#define A(x) /* \\\r\n" + " a comment \\\r\n" + " inside */ \\\r\n" + " f();", + getLLVMStyleWithColumns(17))); +} + TEST_F(FormatTestComments, AlignsBlockComments) { EXPECT_EQ("/*\n" " * Really multi-line\n" Index: lib/Format/BreakableToken.cpp === --- lib/Format/BreakableToken.cpp +++ lib/Format/BreakableToken.cpp @@ -323,7 +323,8 @@ StringRef TokenText(Tok.TokenText); assert(TokenText.startswith("/*") && TokenText.endswith("*/")); - TokenText.substr(2, TokenText.size() - 4).split(Lines, "\n"); + TokenText.substr(2, TokenText.size() - 4) + .split(Lines, TokenText.count('\r') > 0 ? "\r\n" : "\n"); int IndentDelta = StartColumn - OriginalStartColumn; Content.resize(Lines.size()); Index: unittests/Format/FormatTestComments.cpp === --- unittests/Format/FormatTestComments.cpp +++ unittests/Format/FormatTestComments.cpp @@ -472,6 +472,30 @@ " int jjj; /*b*/"); } +TEST_F(FormatTestComments, BlockCommentsWithCLRF) { + EXPECT_EQ("/*\r\n" +" * Comment with\r\n" +"\r\n" +" * blanks.\r\n" +" */\r\n" +"void f() {}", +format("/* \r\n" +" * Comment with\r\n" +" \r\n" +" * blanks.\r\n" +" */\r\n" +"void f() {}")); + EXPECT_EQ("#define A(x) /* \\\r\n" +" a comment \\\r\n" +" inside */ \\\r\n" +" f();", +format("#define A(x) /* \\\r\n" + " a comment \\\r\n" + " inside */ \\\r\n" + " f();", + getLLVMStyleWithColumns(17))); +} + TEST_F(FormatTestComments, AlignsBlockComments) { EXPECT_EQ("/*\n" " * Really multi-line\n" Index: lib/Format/BreakableToken.cpp === --- lib/Format/BreakableToken.cpp +++ lib/Format/BreakableToken.cpp @@ -323,7 +323,8 @@ StringRef TokenText(Tok.TokenText); assert(TokenText.startswith("/*") && TokenText.endswith("*/")); - TokenText.substr(2, TokenText.size() - 4).split(Lines, "\n"); + TokenText.substr(2, TokenText.size() - 4) + .split(Lines, TokenText.count('\r') > 0 ? "\r\n" : "\n"); int IndentDelta = StartColumn - OriginalStartColumn; Content.resize(Lines.size()); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35110: [Analyzer] Constraint Manager Negates Difference
baloghadamsoftware added a comment. Sorry, Artem, but it does not work this way. Even if the symbolic expressions are constrained to `[-MAX/4..MAX/4]`, after rearrangement the difference still uses the whole range, thus `m>n` becomes `m-n>0`, where in the false branch the range for `m-n` is `[MIN..0]`. Then if we check `n>=m` the range set is reversed to `[MIN..MIN]U[0..MAX]` which results `UNKNOWN` for `n-m`. It does not solve any of our problems and there is no remedy on the checker's side. https://reviews.llvm.org/D35110 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47578: Do not enforce absolute path argv0 in windows
takuto.ikuta created this revision. takuto.ikuta added reviewers: thakis, rnk. Herald added a subscriber: hiraditya. Herald added a reviewer: alexshap. Even if we support no-canonical-prefix on clang-cl(https://reviews.llvm.org/D47480), argv0 becomes absolute path in clang-cl and that embeds absolute path in /showIncludes. This patch removes such full path normalization from InitLLVM on windows. Also this patch revealed that some tests may use binary out of build dir. https://reviews.llvm.org/D47578 Files: clang/test/lit.cfg.py lld/test/lit.cfg.py llvm/lib/Support/Windows/Process.inc llvm/test/lit.cfg.py Index: llvm/test/lit.cfg.py === --- llvm/test/lit.cfg.py +++ llvm/test/lit.cfg.py @@ -142,10 +142,10 @@ tools.extend([ 'dsymutil', 'lli', 'lli-child-target', 'llvm-ar', 'llvm-as', 'llvm-bcanalyzer', 'llvm-config', 'llvm-cov', 'llvm-cxxdump', 'llvm-cvtres', 'llvm-diff', 'llvm-dis', -'llvm-dwarfdump', 'llvm-extract', 'llvm-isel-fuzzer', 'llvm-opt-fuzzer', 'llvm-lib', -'llvm-link', 'llvm-lto', 'llvm-lto2', 'llvm-mc', 'llvm-mca', -'llvm-modextract', 'llvm-nm', 'llvm-objcopy', 'llvm-objdump', -'llvm-pdbutil', 'llvm-profdata', 'llvm-ranlib', 'llvm-readobj', +'llvm-dlltool', 'llvm-dwarfdump', 'llvm-dwp', 'llvm-extract', 'llvm-isel-fuzzer', +'llvm-opt-fuzzer', 'llvm-lib', 'llvm-link', 'llvm-lto', 'llvm-lto2', 'llvm-mc', 'llvm-mca', +'llvm-modextract', 'llvm-nm', 'llvm-objcopy', 'llvm-objdump', 'llvm-opt-report', +'llvm-pdbutil', 'llvm-profdata', 'llvm-ranlib', 'llvm-rc', 'llvm-readelf', 'llvm-readobj', 'llvm-rtdyld', 'llvm-size', 'llvm-split', 'llvm-strings', 'llvm-strip', 'llvm-tblgen', 'llvm-c-test', 'llvm-cxxfilt', 'llvm-xray', 'yaml2obj', 'obj2yaml', 'yaml-bench', 'verify-uselistorder', Index: llvm/lib/Support/Windows/Process.inc === --- llvm/lib/Support/Windows/Process.inc +++ llvm/lib/Support/Windows/Process.inc @@ -235,19 +235,11 @@ return mapWindowsError(::GetLastError()); Args.reserve(ArgCount); - std::error_code ec; - - // The first argument may contain just the name of the executable (e.g., - // "clang") rather than the full path, so swap it with the full path. - wchar_t ModuleName[MAX_PATH]; - int Length = ::GetModuleFileNameW(NULL, ModuleName, MAX_PATH); - if (0 < Length && Length < MAX_PATH) -UnicodeCommandLine[0] = ModuleName; // If the first argument is a shortened (8.3) name (which is possible even // if we got the module name), the driver will have trouble distinguishing it // (e.g., clang.exe v. clang++.exe), so expand it now. - ec = ExpandShortFileName(UnicodeCommandLine[0], Args, Alloc); + std::error_code ec = ExpandShortFileName(UnicodeCommandLine[0], Args, Alloc); for (int i = 1; i < ArgCount && !ec; ++i) { ec = WildcardExpand(UnicodeCommandLine[i], Args, Alloc); Index: lld/test/lit.cfg.py === --- lld/test/lit.cfg.py +++ lld/test/lit.cfg.py @@ -39,9 +39,12 @@ llvm_config.use_lld() tool_patterns = [ -'llc', 'llvm-as', 'llvm-mc', 'llvm-nm', 'llvm-objdump', 'llvm-pdbutil', -'llvm-dwarfdump', 'llvm-readelf', 'llvm-readobj', 'obj2yaml', 'yaml2obj', -'opt', 'llvm-dis'] +'llc', 'llvm-ar', 'llvm-as', 'llvm-bcanalyzer', 'llvm-lib', 'llvm-mc', 'llvm-nm', +'llvm-objdump', 'llvm-pdbutil', 'llvm-dwarfdump', 'llvm-readelf', 'llvm-readobj', +'obj2yaml', 'yaml2obj', 'opt', 'llvm-dis'] + +if platform.system() == 'Windows': +tool_patterns += ['LLD-LINK'] llvm_config.add_tool_substitutions(tool_patterns) Index: clang/test/lit.cfg.py === --- clang/test/lit.cfg.py +++ clang/test/lit.cfg.py @@ -58,7 +58,8 @@ tools = [ 'c-index-test', 'clang-check', 'clang-diff', 'clang-format', 'clang-tblgen', -'opt', +'llc', 'llvm-bcanalyzer', 'llvm-dis', 'llvm-lto', 'llvm-nm', 'llvm-objdump', +'llvm-profdata', 'llvm-readobj', 'opt', ToolSubst('%clang_func_map', command=FindTool( 'clang-func-mapping'), unresolved='ignore'), ] Index: llvm/test/lit.cfg.py === --- llvm/test/lit.cfg.py +++ llvm/test/lit.cfg.py @@ -142,10 +142,10 @@ tools.extend([ 'dsymutil', 'lli', 'lli-child-target', 'llvm-ar', 'llvm-as', 'llvm-bcanalyzer', 'llvm-config', 'llvm-cov', 'llvm-cxxdump', 'llvm-cvtres', 'llvm-diff', 'llvm-dis', -'llvm-dwarfdump', 'llvm-extract', 'llvm-isel-fuzzer', 'llvm-opt-fuzzer', 'llvm-lib', -'llvm-link', 'llvm-lto', 'llvm-lto2', 'llvm-mc', 'llvm-mca', -'llvm-modextract', 'llvm-nm', 'llvm-objcopy', 'llvm-objdump', -'llvm-pdbutil', 'llvm-profdata', 'llvm-ranlib', 'llvm-readobj', +'llvm-dlltool', 'llvm-dwarfdump', 'llvm-dwp', 'llvm-extract', 'llvm-isel-fuzzer', +'llvm-opt-fuzzer', 'llvm
[PATCH] D47121: [NEON] Support VLD1xN intrinsics in AArch32 mode (Clang part)
SjoerdMeijer accepted this revision. SjoerdMeijer added a comment. This revision is now accepted and ready to land. I agree: these intrinsics are available in v7/A32/A64. Comment at: lib/CodeGen/CGBuiltin.cpp:7865 } // FIXME: Sharing loads & stores with 32-bit is complicated by the absence // of an Align parameter here. kosarev wrote: > SjoerdMeijer wrote: > > How about this FIXME? Is it still relevant? And does it need to be moved > > up? Or perhaps better: move the code back here to minimise the diff? > Yes, it's still true for the vst builtins handled below. None of the vld/vst > patches removes this comment, but it should go away with whatever is the one > to be committed last. > > Umm, it seems leaving the vld code here wouldn't make the diff smaller? ah yes, nevermind, got confused here. https://reviews.llvm.org/D47121 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35110: [Analyzer] Constraint Manager Negates Difference
baloghadamsoftware added a comment. Maybe if we could apply somehow a `[-MAX/2..MAX/2]` constraint to both sides of the rearranged equality in SimpleSValBuilder. https://reviews.llvm.org/D35110 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47157: Warning for framework headers using double quote includes
aaron.ballman added inline comments. Comment at: include/clang/Basic/DiagnosticLexKinds.td:713 +def warn_quoted_include_in_framework_header : Warning< + "double-quoted include \"%0\" in framework header, expected system style include" + >, InGroup, DefaultIgnore; bruno wrote: > dexonsmith wrote: > > bruno wrote: > > > dexonsmith wrote: > > > > aaron.ballman wrote: > > > > > bruno wrote: > > > > > > aaron.ballman wrote: > > > > > > > 80-col limit? > > > > > > > > > > > > > > Also, I'd probably drop "system style" and reword slightly to: > > > > > > > > > > > > > > `"double-quoted include \"%0\" in framework header, expected > > > > > > > angle-bracketed include <%0> instead"` > > > > > > Unfortunately this won't work because for framework style includes > > > > > > we use the angled-bracketed with the framework name. For example, > > > > > > if one wants to include `Foo.h` from `Foo.framework`, one should > > > > > > use `#include `, although on disk you actually have > > > > > > `Foo.framework/Headers/Foo.h`. Framework header lookup does this > > > > > > magic and other similar ones. > > > > > > > > > > > > Since we don't know which framework the quoted header could be part > > > > > > of, it was not included in the warning (doing so would require > > > > > > extra header searches - which could be expensive for this specific > > > > > > warning). However it seems that I can do better to indicate that > > > > > > the framework name is desired here, perhaps: > > > > > > > > > > > > `"double-quoted include \"%0\" in framework header, expected > > > > > > angle-bracketed include instead"` > > > > > > > > > > > > How does that sound to you? Other suggestions? > > > > > Thank you for the explanation! > > > > > > > > > > I think your suggested text sounds good, though I do wonder how > > > > > expensive is "expensive" in finding the intended header? Not only > > > > > would it provide a better diagnostic, it would also let you use a > > > > > fixit that doesn't use editor placeholders. > > > > I'm also interested in just how expensive it would be, because I think > > > > users will be frustrated that the compiler knows it's a framework > > > > include "so it obviously knows which framework". > > > > > > > > I'd be fine if the fix-it came in a follow-up commit though (not sure > > > > how Aaron feels). > > > I haven't measured, but for each quoted include we would have to: > > > > > > - Start a fresh header search. > > > - Look for `Foo.h` in all possible frameworks in the path (just on the > > > Darwin macOS SDK path that would be around 140 frameworks). > > > - If it's only found in once place, we are mostly safe to say we found a > > > matching framework, otherwise we can't emit a reliable fixit. > > > - Header maps and VFS might add extra level of searches (this is very > > > common in Xcode based clang invocations). > > Can we just check if it's a header in the *same* framework? > For some pretty obvious cases we can probably assume that this is what the > user wants, but even so it might be misleading. For example, if you're in > `ABC/H1.h`, you include `H2.h` and the framework ABC has an `ABC/H2.h`. It > could be that `#include "H2.h"` is mapped via header maps to `$SOURCE/H2.h` > instead of using the installed headers in the framework style build products. > > This is likely a mistake, but what if it's intentional? I would prefer if the > user rethink it instead of just buying potential misleading clues. OTOH, I > share the concern that we don't need to be perfect here and only emit the > fixit for really obvious cases, and not for the others. Will update the patch > to include a fixit to the very straightforward scenario: `H2.h` was found in > the same framework style path as the includer. > Will update the patch to include a fixit to the very straightforward > scenario: H2.h was found in the same framework style path as the includer. Thanks, I think users will appreciate the compiler helping them out like that. Comment at: lib/Lex/HeaderSearch.cpp:753-754 + IncluderAndDir.second->getName())) +Diags.Report(IncludeLoc, + diag::warn_quoted_include_in_framework_header) +<< Filename; dexonsmith wrote: > bruno wrote: > > dexonsmith wrote: > > > bruno wrote: > > > > aaron.ballman wrote: > > > > > This seems like a good place for a fix-it to switch the include > > > > > style. Is there a reason to not do that work for the user? > > > > Like I explained above, we don't know which framework the header could > > > > be part of, so a fix-it could be misleading. > > > Clang supports editor placeholders, which we use in some > > > refactoring-style fix-its. I think this would be spelled > > > `<#framework-name#>`, or `#include <<#framework-name#>/Foo.h>` > > My current understanding (after chatting with @arphaman) is that editor
[PATCH] D44826: Add -Wunused-using, a warning that finds unused using declarations.
CarlosAlbertoEnciso updated this revision to Diff 149261. CarlosAlbertoEnciso added a comment. This patch addresses the reviewers comments: - Additional test cases to cover: -Wunused, -Wall and -Wno-unused-using - Formatting in ReleaseNotes - Removed the '-Wunused-usings' alias (GCC compatibility). - Added individual warning messages for using-declaration, using-directive and namespace-alias. - Fixed SVN attribute changes. https://reviews.llvm.org/D44826 Files: docs/ReleaseNotes.rst include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td include/clang/Sema/ExternalSemaSource.h include/clang/Sema/MultiplexExternalSemaSource.h include/clang/Sema/Sema.h include/clang/Sema/SemaInternal.h include/clang/Serialization/ASTBitCodes.h include/clang/Serialization/ASTReader.h lib/Sema/MultiplexExternalSemaSource.cpp lib/Sema/Sema.cpp lib/Sema/SemaDecl.cpp lib/Sema/SemaDeclCXX.cpp lib/Serialization/ASTReader.cpp lib/Serialization/ASTWriter.cpp test/FixIt/fixit.cpp test/Modules/Inputs/module.map test/Modules/Inputs/warn-unused-using.h test/Modules/warn-unused-using.cpp test/PCH/cxx-templates.cpp test/SemaCXX/coreturn.cpp test/SemaCXX/referenced_alias_declaration_1.cpp test/SemaCXX/referenced_alias_declaration_2.cpp test/SemaCXX/referenced_using_all.cpp test/SemaCXX/referenced_using_declaration_1.cpp test/SemaCXX/referenced_using_declaration_2.cpp test/SemaCXX/referenced_using_directive.cpp test/SemaCXX/referenced_using_options.cpp Index: test/SemaCXX/referenced_using_options.cpp === --- test/SemaCXX/referenced_using_options.cpp +++ test/SemaCXX/referenced_using_options.cpp @@ -0,0 +1,17 @@ +// RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wall -verify %s +// RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wunused -verify %s +// RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wunused-using -verify %s + +namespace N { + typedef int Integer; +} + +using N::Integer; // expected-warning {{unused using declaration 'Integer'}} + +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wunused-using" +using N::Integer; // no warning +#pragma clang diagnostic pop + +using N::Integer; // expected-warning {{unused using declaration 'Integer'}} + Index: test/SemaCXX/referenced_using_directive.cpp === --- test/SemaCXX/referenced_using_directive.cpp +++ test/SemaCXX/referenced_using_directive.cpp @@ -0,0 +1,27 @@ +// RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wunused-using -verify %s + +namespace N { + typedef int Integer; + int var; +} + +void Fa() { + using namespace N; // Referenced + var = 1; +} + +void Fb() { + using namespace N; // expected-warning {{unused using directive 'N'}} + N::var = 1; +} + +void Fc() { + using namespace N; // Referenced + Integer var = 1; +} + +void Fd() { + using namespace N; // expected-warning {{unused using directive 'N'}} + N::Integer var = 1; +} + Index: test/SemaCXX/referenced_using_declaration_2.cpp === --- test/SemaCXX/referenced_using_declaration_2.cpp +++ test/SemaCXX/referenced_using_declaration_2.cpp @@ -0,0 +1,21 @@ +// RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wunused-using -verify %s + +namespace N { + typedef int Integer; + typedef char Char; +} + +using N::Integer; // expected-warning {{unused using declaration 'Integer'}} +using N::Char; // Referenced + +void Foo(int p1, N::Integer p2, Char p3) { + N::Integer var; + var = 0; +} + +using N::Integer; // Referenced +Integer Bar() { + using N::Char;// expected-warning {{unused using declaration 'Char'}} + return 0; +} + Index: test/SemaCXX/referenced_using_declaration_1.cpp === --- test/SemaCXX/referenced_using_declaration_1.cpp +++ test/SemaCXX/referenced_using_declaration_1.cpp @@ -0,0 +1,47 @@ +// RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wunused-using -verify %s + +namespace N { + // Types. + typedef int Integer; + struct Record { +int a; + }; + + // Variables. + int var1; + int var2; + + // Functions. + void func1(); + void func2(); +} + +using N::Integer; // expected-warning {{unused using declaration 'Integer'}} +using N::Record;// expected-warning {{unused using declaration 'Record'}} +using N::var1; // expected-warning {{unused using declaration 'var1'}} +using N::var2; // expected-warning {{unused using declaration 'var2'}} +using N::func1; // expected-warning {{unused using declaration 'func1'}} +using N::func2; // expected-warning {{unused using declaration 'func2'}} + +void Foo() { + using N::Integer; // expected-warning {{unused using declaration 'Integer'}} + N::Integer int_var; + int_var = 1; + + using N::Record; // Referenced + Record rec_var; + rec_var.a = 2; + + using N::var1;// expected-warning {{u
[PATCH] D47474: Implement cpu_dispatch/cpu_specific Multiversioning
aaron.ballman added inline comments. Comment at: include/clang/Basic/Attr.td:851 + let Spellings = [GCC<"cpu_specific">]; + let Args = [VariadicIdentifierArgument<"Cpus">]; + let Subjects = SubjectList<[Function]>; erichkeane wrote: > aaron.ballman wrote: > > Be sure to add a test for using this attribute with the C++ spelling, as > > I'm not certain how well we would parse something like > > `[[gnu::cpu_specific(ivybridge)]]` currently (it may just work, however). > > > > Also, why an identifier instead of a string literal? > I'll add it, presumably as 'clang::cpu_specific'. The decision for > string-literal vs identifier was made quite a few years before I was here > sadly. I believe the EDG FE doesn't make identifiers any more difficult so > the implementer here chose to make it that way. > > In this case, I'd very much like to keep it with the same implementation as > ICC, simply because users of this are already familiar with it in this form. > In this case, I'd very much like to keep it with the same implementation as > ICC, simply because users of this are already familiar with it in this form. The compatibility with ICC is important for the GNU-style attribute, but for the C++ spelling this is novel territory where there is no compatibility story. Another approach to consider is whether to accept identifiers or string literals depending on the spelling, but that might not be worth it. Comment at: include/clang/Basic/AttrDocs.td:225-226 +as a parameter (like ``cpu_specific``). However, ``cpu_dispatch`` functions +may not have a body in its definition. An empty definition is permissible for +ICC compatibility, and all other definitions will have their body ignored. + erichkeane wrote: > aaron.ballman wrote: > > How empty is empty? e.g., is it lexically empty (no tokens whatsoever) or > > notionally empty (empty after preprocessing)? > > ``` > > __attribute__((cpu_dispatch(atom))) void multi_cpu_1(void) { > > /* Is this empty enough? */ > > } > > > > __attribute__((cpu_dispatch(atom))) void multi_cpu_2(void) { > > #if 0 > > #error How about this? > > #endif > > } > > ``` > Well, ANY are permissible (even non-empty bodies...) since I issue a warning > if I detect that it is not empty. I detect it at the AST level, so anything > that doesn't add to the AST isn't counted against 'empty'. In this case, > those two are both empty. > > Do you have a suggestion on how I can word this more clearly? Would this be accurate? ``` Functions marked with ``cpu_dispatch`` are not expected to be defined, only declared. If such a marked function has a definition, any side effects of the function are ignored; trivial function bodies are permissible for ICC compatibility. ``` (Question: if this is true, should you also update `FunctionDecl::hasTrivialBody()` to return `true` for functions with this attribute?) https://reviews.llvm.org/D47474 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46602: [clang-tidy] Store checks profiling info as YAML files
aaron.ballman added inline comments. Comment at: clang-tidy/ClangTidyProfiling.cpp:72 + if (EC) { +llvm::errs() << "Error opening output file'" << Storage->StoreFilename + << "': " << EC.message() << "\n"; Missing a whitespace before the quote for the file name. Comment at: clang-tidy/tool/ClangTidyMain.cpp:334 + auto processPrefix = [](const std::string &input) -> SmallString<256> { +if (input.empty()) The identifiers here don't match the coding convention (should be `ProcessPrefix` and `Input` by convention). Comment at: clang-tidy/tool/ClangTidyMain.cpp:342-347 +if (!llvm::sys::fs::exists(AbsolutePath)) { + // If the destination prefix does not exist, don't try to use real_path(). + return AbsolutePath; +} +SmallString<256> dest; +if (std::error_code EC = llvm::sys::fs::real_path(AbsolutePath, dest)) { This creates a TOCTOU bug; can you call `real_path()` without checking for existence and instead check the error code to decide whether to spit out an error or return `AbsolutePath`? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D46602 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47532: [ASTImporter] Import the whole redecl chain of functions
balazske added inline comments. Comment at: lib/AST/ASTImporter.cpp:88 + llvm::SmallVector getCanonicalForwardRedeclChain(Decl* D) { +// Currently only FunctionDecl is supported +auto FD = cast(D); Assert for FunctionDecl? Comment at: unittests/AST/ASTImporterTest.cpp:1745 +TEST_P(ImportFunctions, ImportDefinitions) { + auto Pattern = functionDecl(hasName("f")); + This test imports the definition two times, the second should result in error. The import does not check the function body for similarness. Probably a check can be made for the result of the second import. Repository: rC Clang https://reviews.llvm.org/D47532 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47067: Update NRVO logic to support early return
tzik added a comment. In https://reviews.llvm.org/D47067#1116733, @rsmith wrote: > Slightly reduced testcase: > > template T f(T u, bool b) { > T t; > if (b) return t; > return u; > } > struct X { X(); X(const X&); ~X(); void *data; }; > > template X f(X, bool); > > > ... which compiles to: > > X f(X u, bool b) { > X::X(&return-slot); > if (b) return; > X::X(&return-slot, u); > X::~X(&return-slot); > } > > > ... due to `t` incorrectly being used as an NRVO variable. Thanks for your investigation! I thought that's covered by Scope::setNRVOCandidate with a null `Candidate`. As `t` is not marked as an NRVO variable before its instantiation. There's likely something I missed that turn the flag on while its instantiation. Repository: rC Clang https://reviews.llvm.org/D47067 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47578: Do not enforce absolute path argv0 in windows
ruiu added a comment. Looks like this patch contains two unrelated changes. Please separate your change from the lit config changes. https://reviews.llvm.org/D47578 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47474: Implement cpu_dispatch/cpu_specific Multiversioning
erichkeane added inline comments. Comment at: include/clang/Basic/Attr.td:851 + let Spellings = [GCC<"cpu_specific">]; + let Args = [VariadicIdentifierArgument<"Cpus">]; + let Subjects = SubjectList<[Function]>; aaron.ballman wrote: > erichkeane wrote: > > aaron.ballman wrote: > > > Be sure to add a test for using this attribute with the C++ spelling, as > > > I'm not certain how well we would parse something like > > > `[[gnu::cpu_specific(ivybridge)]]` currently (it may just work, however). > > > > > > Also, why an identifier instead of a string literal? > > I'll add it, presumably as 'clang::cpu_specific'. The decision for > > string-literal vs identifier was made quite a few years before I was here > > sadly. I believe the EDG FE doesn't make identifiers any more difficult so > > the implementer here chose to make it that way. > > > > In this case, I'd very much like to keep it with the same implementation as > > ICC, simply because users of this are already familiar with it in this form. > > In this case, I'd very much like to keep it with the same implementation as > > ICC, simply because users of this are already familiar with it in this form. > > The compatibility with ICC is important for the GNU-style attribute, but for > the C++ spelling this is novel territory where there is no compatibility > story. Another approach to consider is whether to accept identifiers or > string literals depending on the spelling, but that might not be worth it. I'd like to think about that... I could forsee accepting BOTH forms, simply because it would slightly simplify the conversion from an attribute-target-mv situation, though I'm not sure it is important enough to do. Comment at: include/clang/Basic/AttrDocs.td:225-226 +as a parameter (like ``cpu_specific``). However, ``cpu_dispatch`` functions +may not have a body in its definition. An empty definition is permissible for +ICC compatibility, and all other definitions will have their body ignored. + aaron.ballman wrote: > erichkeane wrote: > > aaron.ballman wrote: > > > How empty is empty? e.g., is it lexically empty (no tokens whatsoever) or > > > notionally empty (empty after preprocessing)? > > > ``` > > > __attribute__((cpu_dispatch(atom))) void multi_cpu_1(void) { > > > /* Is this empty enough? */ > > > } > > > > > > __attribute__((cpu_dispatch(atom))) void multi_cpu_2(void) { > > > #if 0 > > > #error How about this? > > > #endif > > > } > > > ``` > > Well, ANY are permissible (even non-empty bodies...) since I issue a > > warning if I detect that it is not empty. I detect it at the AST level, so > > anything that doesn't add to the AST isn't counted against 'empty'. In > > this case, those two are both empty. > > > > Do you have a suggestion on how I can word this more clearly? > Would this be accurate? > ``` > Functions marked with ``cpu_dispatch`` are not expected to be defined, only > declared. If such a marked function has a definition, any side effects of the > function are ignored; trivial function bodies are permissible for ICC > compatibility. > ``` > (Question: if this is true, should you also update > `FunctionDecl::hasTrivialBody()` to return `true` for functions with this > attribute?) That is accurate and elegantly describes the behavior. I don't think it makes sense to mark the body 'trivial' since it actually WILL have a body, its just emitted based on the attribute rather than the user-defined body. I'd also fear some of the side-effects of permitting someone to detect it as 'trivial' for SFINAE purposes. https://reviews.llvm.org/D47474 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47474: Implement cpu_dispatch/cpu_specific Multiversioning
erichkeane updated this revision to Diff 149276. erichkeane added a comment. Shamelessly stealing @aaron.ballman s AttrDocs description of cpu_dispatch bodies. https://reviews.llvm.org/D47474 Files: include/clang/AST/Decl.h include/clang/Basic/Attr.td include/clang/Basic/AttrDocs.td include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td include/clang/Basic/TargetInfo.h include/clang/Basic/X86Target.def lib/AST/Decl.cpp lib/Basic/Targets/X86.cpp lib/Basic/Targets/X86.h lib/CodeGen/CGBuiltin.cpp lib/CodeGen/CodeGenFunction.cpp lib/CodeGen/CodeGenFunction.h lib/CodeGen/CodeGenModule.cpp lib/CodeGen/CodeGenModule.h lib/Parse/ParseDecl.cpp lib/Sema/AnalysisBasedWarnings.cpp lib/Sema/Sema.cpp lib/Sema/SemaDecl.cpp lib/Sema/SemaDeclAttr.cpp lib/Sema/SemaExpr.cpp lib/Sema/SemaOverload.cpp test/CodeGen/attr-cpuspecific.c test/Misc/pragma-attribute-supported-attributes-list.test test/Sema/attr-cpuspecific.c test/SemaCXX/attr-cpuspecific.cpp utils/TableGen/ClangAttrEmitter.cpp Index: utils/TableGen/ClangAttrEmitter.cpp === --- utils/TableGen/ClangAttrEmitter.cpp +++ utils/TableGen/ClangAttrEmitter.cpp @@ -1173,6 +1173,13 @@ } }; + class VariadicIdentifierArgument : public VariadicArgument { + public: +VariadicIdentifierArgument(const Record &Arg, StringRef Attr) + : VariadicArgument(Arg, Attr, "IdentifierInfo *") +{} + }; + class VariadicStringArgument : public VariadicArgument { public: VariadicStringArgument(const Record &Arg, StringRef Attr) @@ -1278,6 +1285,8 @@ Ptr = llvm::make_unique(Arg, Attr); else if (ArgName == "ParamIdxArgument") Ptr = llvm::make_unique(Arg, Attr, "ParamIdx"); + else if (ArgName == "VariadicIdentifierArgument") +Ptr = llvm::make_unique(Arg, Attr); else if (ArgName == "VersionArgument") Ptr = llvm::make_unique(Arg, Attr); @@ -2106,6 +2115,34 @@ .Default(false); } +static bool isVariadicIdentifierArgument(Record *Arg) { + return !Arg->getSuperClasses().empty() && + llvm::StringSwitch( + Arg->getSuperClasses().back().first->getName()) + .Case("VariadicIdentifierArgument", true) + .Default(false); +} + +static void emitClangAttrVariadicIdentifierArgList(RecordKeeper &Records, + raw_ostream &OS) { + OS << "#if defined(CLANG_ATTR_VARIADIC_IDENTIFIER_ARG_LIST)\n"; + std::vector Attrs = Records.getAllDerivedDefinitions("Attr"); + for (const auto *A : Attrs) { +// Determine whether the first argument is a variadic identifier. +std::vector Args = A->getValueAsListOfDefs("Args"); +if (Args.empty() || !isVariadicIdentifierArgument(Args[0])) + continue; + +// All these spellings take an identifier argument. +forEachUniqueSpelling(*A, [&](const FlattenedSpelling &S) { + OS << ".Case(\"" << S.name() << "\", " + << "true" + << ")\n"; +}); + } + OS << "#endif // CLANG_ATTR_VARIADIC_IDENTIFIER_ARG_LIST\n\n"; +} + // Emits the first-argument-is-identifier property for attributes. static void emitClangAttrIdentifierArgList(RecordKeeper &Records, raw_ostream &OS) { OS << "#if defined(CLANG_ATTR_IDENTIFIER_ARG_LIST)\n"; @@ -3696,6 +3733,7 @@ emitSourceFileHeader("Parser-related llvm::StringSwitch cases", OS); emitClangAttrArgContextList(Records, OS); emitClangAttrIdentifierArgList(Records, OS); + emitClangAttrVariadicIdentifierArgList(Records, OS); emitClangAttrTypeArgList(Records, OS); emitClangAttrLateParsedList(Records, OS); } Index: test/SemaCXX/attr-cpuspecific.cpp === --- /dev/null +++ test/SemaCXX/attr-cpuspecific.cpp @@ -0,0 +1,111 @@ +// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -verify -fexceptions -fcxx-exceptions %s -std=c++14 + +// expected-error@+1{{invalid option 'invalid' for cpu_dispatch}} +void __attribute__((cpu_dispatch(atom, invalid))) invalid_cpu(); + +void __attribute__((cpu_specific(atom))) no_default(void); +void __attribute__((cpu_specific(sandybridge))) no_default(void); + +struct MVReference { + int __attribute__((cpu_specific(sandybridge))) bar(void); + int __attribute__((cpu_specific(ivybridge))) bar(void); + int __attribute__((cpu_specific(sandybridge))) foo(void); +}; + +void use1(void){ + // OK, will fail in the linker, unless another TU provides the cpu_dispatch. + no_default(); + + // expected-error@+1 {{call to non-static member function without an object argument}} + +MVReference::bar; + // expected-error@+1 {{call to non-static member function without an object argument}} + +MVReference::foo; + // expected-error@+1 {{reference to multiversioned function could not be resolved; did you mean to call it?}} + &MVReference::bar; + // expected-error@+1 {{reference to multiversioned function could not b
[PATCH] D47586: Update NRVO logic to support early return (Attempt 2)
tzik created this revision. Herald added a subscriber: cfe-commits. This is the second attempt of r333500 (Update NRVO logic to support early return). The previous one was reverted for a miscompilation for an incorrect NRVO set up on templates such as: struct Foo {}; template T bar() { T t; if (false) return T(); return t; } Where, `t` is marked as non-NRVO variable before its instantiation. However, while its instantiation, it's left an NRVO candidate, turned into an NRVO variable later. Repository: rC Clang https://reviews.llvm.org/D47586 Files: include/clang/AST/Decl.h include/clang/Sema/Scope.h lib/Sema/Scope.cpp lib/Sema/SemaDecl.cpp lib/Sema/SemaExpr.cpp lib/Sema/SemaStmt.cpp lib/Sema/SemaTemplateInstantiateDecl.cpp lib/Serialization/ASTReaderDecl.cpp lib/Serialization/ASTWriterDecl.cpp test/CodeGenCXX/nrvo-noopt.cpp test/CodeGenCXX/nrvo.cpp test/SemaCXX/nrvo-ast.cpp Index: test/SemaCXX/nrvo-ast.cpp === --- /dev/null +++ test/SemaCXX/nrvo-ast.cpp @@ -0,0 +1,139 @@ +// RUN: %clang_cc1 -fcxx-exceptions -fsyntax-only -ast-dump -o - %s | FileCheck %s + +struct X { + X(); + X(const X&); + X(X&&); +}; + +// CHECK-LABEL: FunctionDecl {{.*}} test_00 +X test_00() { + // CHECK: VarDecl {{.*}} x {{.*}} nrvo + X x; + return x; +} + +// CHECK-LABEL: FunctionDecl {{.*}} test_01 +X test_01(bool b) { + // CHECK: VarDecl {{.*}} x {{.*}} nrvo + X x; + if (b) +return x; + return x; +} + +// CHECK-LABEL: FunctionDecl {{.*}} test_02 +X test_02(bool b) { + // CHECK-NOT: VarDecl {{.*}} x {{.*}} nrvo + X x; + // CHECK-NOT: VarDecl {{.*}} y {{.*}} nrvo + X y; + if (b) +return y; + return x; +} + +// CHECK-LABEL: FunctionDecl {{.*}} test_03 +X test_03(bool b) { + if (b) { +// CHECK: VarDecl {{.*}} y {{.*}} nrvo +X y; +return y; + } + // CHECK: VarDecl {{.*}} x {{.*}} nrvo + X x; + return x; +} + +extern "C" _Noreturn void exit(int) throw(); + +// CHECK-LABEL: FunctionDecl {{.*}} test_04 +X test_04(bool b) { + { +// CHECK: VarDecl {{.*}} x {{.*}} nrvo +X x; +if (b) + return x; + } + exit(1); +} + +void may_throw(); +// CHECK-LABEL: FunctionDecl {{.*}} test_05 +X test_05() { + try { +may_throw(); +return X(); + } catch (X x) { +// CHECK-NOT: VarDecl {{.*}} x {{.*}} nrvo +return x; + } +} + +// CHECK-LABEL: FunctionDecl {{.*}} test_06 +X test_06() { + // CHECK-NOT: VarDecl {{.*}} x {{.*}} nrvo + X x __attribute__((aligned(8))); + return x; +} + +// CHECK-LABEL: FunctionDecl {{.*}} test_07 +X test_07(bool b) { + if (b) { +// CHECK: VarDecl {{.*}} x {{.*}} nrvo +X x; +return x; + } + return X(); +} + +// CHECK-LABEL: FunctionDecl {{.*}} test_08 +X test_08(bool b) { + if (b) { +// CHECK: VarDecl {{.*}} x {{.*}} nrvo +X x; +return x; + } else { +// CHECK: VarDecl {{.*}} y {{.*}} nrvo +X y; +return y; + } +} + +template +struct Y { + Y(); + // CHECK-LABEL: CXXMethodDecl {{.*}} test_09 'Y ()' + // CHECK: VarDecl {{.*}} y 'Y' nrvo + + // CHECK-LABEL: CXXMethodDecl {{.*}} test_09 'Y ()' + // CHECK: VarDecl {{.*}} y 'Y' nrvo + static Y test_09() { +Y y; +return y; + } +}; + +struct Z { + Z(const X&); +}; + +// CHECK-LABEL: FunctionDecl {{.*}} test_10 'A ()' +// CHECK: VarDecl {{.*}} b 'B' nrvo + +// CHECK-LABEL: FunctionDecl {{.*}} test_10 'X ()' +// CHECK: VarDecl {{.*}} b {{.*}} nrvo + +// CHECK-LABEL: FunctionDecl {{.*}} test_10 'Z ()' +// CHECK-NOT: VarDecl {{.*}} b {{.*}} nrvo +template +A test_10() { + B b; + return b; +} + +void instantiate() { + Y::test_09(); + test_10(); + test_10(); +} Index: test/CodeGenCXX/nrvo.cpp === --- test/CodeGenCXX/nrvo.cpp +++ test/CodeGenCXX/nrvo.cpp @@ -130,17 +130,13 @@ } // CHECK-LABEL: define void @_Z5test3b -X test3(bool B) { +X test3(bool B, X x) { // CHECK: tail call {{.*}} @_ZN1XC1Ev - // CHECK-NOT: call {{.*}} @_ZN1XC1ERKS_ - // CHECK: call {{.*}} @_ZN1XC1Ev - // CHECK: call {{.*}} @_ZN1XC1ERKS_ if (B) { X y; return y; } - // FIXME: we should NRVO this variable too. - X x; + // CHECK: tail call {{.*}} @_ZN1XC1ERKS_ return x; } @@ -191,21 +187,29 @@ } // CHECK-LABEL: define void @_Z5test7b +// CHECK-EH-LABEL: define void @_Z5test7b X test7(bool b) { // CHECK: tail call {{.*}} @_ZN1XC1Ev // CHECK-NEXT: ret + + // CHECK-EH: tail call {{.*}} @_ZN1XC1Ev + // CHECK-EH-NEXT: ret if (b) { X x; return x; } return X(); } // CHECK-LABEL: define void @_Z5test8b +// CHECK-EH-LABEL: define void @_Z5test8b X test8(bool b) { // CHECK: tail call {{.*}} @_ZN1XC1Ev // CHECK-NEXT: ret - if (b) { + + // CHECK-EH: tail call {{.*}} @_ZN1XC1Ev + // CHECK-EH-NEXT: ret +if (b) { X x; return x; } else { @@ -221,4 +225,37 @@ // CHECK-LABEL: define linkonce_odr void @_ZN1YIiE1fEv // CHECK
[PATCH] D47532: [ASTImporter] Import the whole redecl chain of functions
martong added inline comments. Comment at: lib/AST/ASTImporter.cpp:88 + llvm::SmallVector getCanonicalForwardRedeclChain(Decl* D) { +// Currently only FunctionDecl is supported +auto FD = cast(D); balazske wrote: > Assert for FunctionDecl? `cast` itself will assert if it is not a `FunctionDecl`. (`dyn_cast` is the other formula which can result with a nullptr.) Comment at: unittests/AST/ASTImporterTest.cpp:1745 +TEST_P(ImportFunctions, ImportDefinitions) { + auto Pattern = functionDecl(hasName("f")); + balazske wrote: > This test imports the definition two times, the second should result in > error. The import does not check the function body for similarness. Probably > a check can be made for the result of the second import. I don't think this should result an error, because the second definition has the very same type as the first. And also the body is structurally equivalent too, but we do not check that. Perhaps, on a long term we should check the body too (?). I think the importer should work in this way: if there is already a definition then just simply use that and omit any subsequent definitions if they are the same definitions (or report error if the body is different). The key principle I followed is this: we can have arbitrary number of prototypes but only one definition. Repository: rC Clang https://reviews.llvm.org/D47532 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r333653 - Add a new driver mode to dump compiler feature and extension options.
Author: aaronballman Date: Thu May 31 06:57:09 2018 New Revision: 333653 URL: http://llvm.org/viewvc/llvm-project?rev=333653&view=rev Log: Add a new driver mode to dump compiler feature and extension options. Add the ability to dump compiler option-related information to a JSON file via the -compiler-options-dump option. Specifically, it dumps the features/extensions lists -- however, this output could be extended to other information should it be useful. In order to support features and extensions, I moved them into a .def file so that we could build the various lists we care about from them without a significant increase in maintenance burden. Added: cfe/trunk/include/clang/Basic/Features.def cfe/trunk/test/Frontend/compiler-options-dump.cpp Modified: cfe/trunk/include/clang/Driver/CC1Options.td cfe/trunk/include/clang/Frontend/FrontendActions.h cfe/trunk/include/clang/Frontend/FrontendOptions.h cfe/trunk/lib/Frontend/CompilerInvocation.cpp cfe/trunk/lib/Frontend/FrontendActions.cpp cfe/trunk/lib/FrontendTool/ExecuteCompilerInvocation.cpp cfe/trunk/lib/Lex/PPMacroExpansion.cpp Added: cfe/trunk/include/clang/Basic/Features.def URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Features.def?rev=333653&view=auto == --- cfe/trunk/include/clang/Basic/Features.def (added) +++ cfe/trunk/include/clang/Basic/Features.def Thu May 31 06:57:09 2018 @@ -0,0 +1,238 @@ +//===--- Features.def - Features and Extensions database *- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// +// +// This file defines features exposed via __has_feature and extensions exposed +// via __has_extension. Users of this file must either define the FEATURE or +// EXTENSION macros (or both) to make use of this information. Note that these +// macros expect the following declarations to be available for the Predicate: +// +// const LangOptions &LangOpts; +// const Preprocessor &PP; +// +// The Predicate field dictates the conditions under which the feature or +// extension will be made available. +//===--===// + +#if !defined(FEATURE) && !defined(EXTENSION) +# error Define either the FEATURE or EXTENSION macro to handle features +#endif + +#ifndef FEATURE +#define FEATURE(Name, Predicate) +#endif + +#ifndef EXTENSION +#define EXTENSION(Name, Predicate) +#endif + +FEATURE(address_sanitizer, +LangOpts.Sanitize.hasOneOf(SanitizerKind::Address | + SanitizerKind::KernelAddress)) +FEATURE(hwaddress_sanitizer, +LangOpts.Sanitize.hasOneOf(SanitizerKind::HWAddress | + SanitizerKind::KernelHWAddress)) +FEATURE(assume_nonnull, true) +FEATURE(attribute_analyzer_noreturn, true) +FEATURE(attribute_availability, true) +FEATURE(attribute_availability_with_message, true) +FEATURE(attribute_availability_app_extension, true) +FEATURE(attribute_availability_with_version_underscores, true) +FEATURE(attribute_availability_tvos, true) +FEATURE(attribute_availability_watchos, true) +FEATURE(attribute_availability_with_strict, true) +FEATURE(attribute_availability_with_replacement, true) +FEATURE(attribute_availability_in_templates, true) +FEATURE(attribute_cf_returns_not_retained, true) +FEATURE(attribute_cf_returns_retained, true) +FEATURE(attribute_cf_returns_on_parameters, true) +FEATURE(attribute_deprecated_with_message, true) +FEATURE(attribute_deprecated_with_replacement, true) +FEATURE(attribute_ext_vector_type, true) +FEATURE(attribute_ns_returns_not_retained, true) +FEATURE(attribute_ns_returns_retained, true) +FEATURE(attribute_ns_consumes_self, true) +FEATURE(attribute_ns_consumed, true) +FEATURE(attribute_cf_consumed, true) +FEATURE(attribute_objc_ivar_unused, true) +FEATURE(attribute_objc_method_family, true) +FEATURE(attribute_overloadable, true) +FEATURE(attribute_unavailable_with_message, true) +FEATURE(attribute_unused_on_fields, true) +FEATURE(attribute_diagnose_if_objc, true) +FEATURE(blocks, LangOpts.Blocks) +FEATURE(c_thread_safety_attributes, true) +FEATURE(cxx_exceptions, LangOpts.CXXExceptions) +FEATURE(cxx_rtti, LangOpts.RTTI &&LangOpts.RTTIData) +FEATURE(enumerator_attributes, true) +FEATURE(nullability, true) +FEATURE(nullability_on_arrays, true) +FEATURE(memory_sanitizer, LangOpts.Sanitize.has(SanitizerKind::Memory)) +FEATURE(thread_sanitizer, LangOpts.Sanitize.has(SanitizerKind::Thread)) +FEATURE(dataflow_sanitizer, LangOpts.Sanitize.has(SanitizerKind::DataFlow)) +FEATURE(efficiency_sanitizer, +LangOpts.Sanitize.hasOneOf(SanitizerKind::Efficiency)) +FEATURE(scudo, LangOpts.Sanitize.hasOneOf(SanitizerKin
[PATCH] D45835: Add new driver mode for dumping compiler options
aaron.ballman closed this revision. aaron.ballman marked 4 inline comments as done. aaron.ballman added a comment. Committed in r333653. Comment at: test/Frontend/compiler-options-dump.cpp:3 +// RUN: %clang_cc1 -compiler-options-dump -std=c++17 %s -o - | FileCheck %s --check-prefix=CXX17 +// RUN: %clang_cc1 -compiler-options-dump -std=c99 -ffast-math -x c %s -o - | FileCheck %s --check-prefix=C99 + aaron.ballman wrote: > hfinkel wrote: > > You don't need -ffast-math here I presume. > Correct, I'll drop from the commit (or in the next patch). I dropped this from the commit. https://reviews.llvm.org/D45835 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36918: [Sema] Take into account the current context when checking the accessibility of a member function pointer
ahatanak added a comment. ping Repository: rC Clang https://reviews.llvm.org/D36918 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44826: Add -Wunused-using, a warning that finds unused using declarations.
thakis accepted this revision. thakis added a comment. This revision is now accepted and ready to land. Looks great. Comment at: docs/ReleaseNotes.rst:126 +namespace n { class C; } +using n::C; // Never actually used. + Maybe include an example with a using directive and a namespace alias as well. Comment at: docs/ReleaseNotes.rst:139 + used in conjunction with ``-Werror`` and as a result, the new warnings + are turned into new errors. + nit: I'd omit this paragraph -- this is true for all warnings and not special for this warning. Comment at: include/clang/Basic/DiagnosticGroups.td:828-829 // -Wunused-local-typedefs = -Wunused-local-typedef +def : DiagGroup<"unused-usings", [UnusedUsing]>; +// -Wunused-usings = -Wunused-using CarlosAlbertoEnciso wrote: > lebedev.ri wrote: > > Why? gcc compatibility? > No particular reason. I just follow the 'unused-local-typedefs' model. > If there is not objection from others reviewers, I will drop the gcc > compatibility. Does gcc have a `-Wunused-usings`? As far as I can tell it doesn't, so I agree not having the alias makes sense. -Wunused-local-typedefs is here because gcc has that flag. https://reviews.llvm.org/D44826 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call
Szelethus updated this revision to Diff 149290. Szelethus added a comment. - Rebased to 1cefbc5593d2f017ae56a853b0723a31865aa602 (revision 333276) - Fixed some typos - Added tests `CyclicPointerTest` and `CyclicVoidPointerTest` to highlight an issue to be fixed in a later patch https://reviews.llvm.org/D45532 Files: include/clang/StaticAnalyzer/Checkers/Checkers.td lib/StaticAnalyzer/Checkers/CMakeLists.txt lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp test/Analysis/Inputs/system-header-simulator-for-cxx-uninitialized-object.h test/Analysis/cxx-uninitialized-object-inheritance.cpp test/Analysis/cxx-uninitialized-object-ptr-ref.cpp test/Analysis/cxx-uninitialized-object.cpp Index: test/Analysis/cxx-uninitialized-object.cpp === --- /dev/null +++ test/Analysis/cxx-uninitialized-object.cpp @@ -0,0 +1,1058 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.cplusplus.UninitializedObject -analyzer-config alpha.cplusplus.UninitializedObject:Pedantic=true -std=c++11 -DPEDANTIC -verify %s + +// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.cplusplus.UninitializedObject -std=c++11 -verify %s + +//===--===// +// Default constructor test. +//===--===// + +class CompilerGeneratedConstructorTest { + int a, b, c, d, e, f, g, h, i, j; + +public: + CompilerGeneratedConstructorTest() = default; +}; + +void fCompilerGeneratedConstructorTest() { + CompilerGeneratedConstructorTest(); +} + +#ifdef PEDANTIC +class DefaultConstructorTest { + int a; // expected-note{{uninitialized field 'this->a'}} + +public: + DefaultConstructorTest(); +}; + +DefaultConstructorTest::DefaultConstructorTest() = default; + +void fDefaultConstructorTest() { + DefaultConstructorTest(); // expected-warning{{1 uninitialized field}} +} +#else +class DefaultConstructorTest { + int a; + +public: + DefaultConstructorTest(); +}; + +DefaultConstructorTest::DefaultConstructorTest() = default; + +void fDefaultConstructorTest() { + DefaultConstructorTest(); +} +#endif // PEDANTIC + +//===--===// +// Initializer list test. +//===--===// + +class InitListTest1 { + int a; + int b; + +public: + InitListTest1() + : a(1), +b(2) { +// All good! + } +}; + +void fInitListTest1() { + InitListTest1(); +} + +class InitListTest2 { + int a; + int b; // expected-note{{uninitialized field 'this->b'}} + +public: + InitListTest2() + : a(3) {} // expected-warning{{1 uninitialized field}} +}; + +void fInitListTest2() { + InitListTest2(); +} + +class InitListTest3 { + int a; // expected-note{{uninitialized field 'this->a'}} + int b; + +public: + InitListTest3() + : b(4) {} // expected-warning{{1 uninitialized field}} +}; + +void fInitListTest3() { + InitListTest3(); +} + +//===--===// +// Constructor body test. +//===--===// + +class CtorBodyTest1 { + int a, b; + +public: + CtorBodyTest1() { +a = 5; +b = 6; +// All good! + } +}; + +void fCtorBodyTest1() { + CtorBodyTest1(); +} + +class CtorBodyTest2 { + int a; + int b; // expected-note{{uninitialized field 'this->b'}} + +public: + CtorBodyTest2() { +a = 7; // expected-warning{{1 uninitialized field}} + } +}; + +void fCtorBodyTest2() { + CtorBodyTest2(); +} + +class CtorBodyTest3 { + int a; // expected-note{{uninitialized field 'this->a'}} + int b; + +public: + CtorBodyTest3() { +b = 8; // expected-warning{{1 uninitialized field}} + } +}; + +void fCtorBodyTest3() { + CtorBodyTest3(); +} + +#ifdef PEDANTIC +class CtorBodyTest4 { + int a; // expected-note{{uninitialized field 'this->a'}} + int b; // expected-note{{uninitialized field 'this->b'}} + +public: + CtorBodyTest4() {} +}; + +void fCtorBodyTest4() { + CtorBodyTest4(); // expected-warning{{2 uninitialized fields}} +} +#else +class CtorBodyTest4 { + int a; + int b; + +public: + CtorBodyTest4() {} +}; + +void fCtorBodyTest4() { + CtorBodyTest4(); +} +#endif + +//===--===// +// Constructor delegation test. +//===--===// + +class CtorDelegationTest1 { + int a; + int b; + +public: + CtorDelegationTest1(int) + : a(9) { +// leaves 'b' unintialized, but we'll never check this function + } + + CtorDelegationTest1() + : CtorDelegationTest1(int{}) { // Initializing 'a' +b = 10; +// All good! + } +}; + +void fCtorDelegationTest1() { + CtorDelegationTest1(); +} + +class CtorDelegationTest2 { + int a; // expected-note{{uninitializ
r333657 - Add dump method for selectors
Author: hiraditya Date: Thu May 31 07:45:32 2018 New Revision: 333657 URL: http://llvm.org/viewvc/llvm-project?rev=333657&view=rev Log: Add dump method for selectors Differential Revision: https://reviews.llvm.org/D45935 Reviewers: compnerd Modified: cfe/trunk/include/clang/Basic/IdentifierTable.h cfe/trunk/lib/Basic/IdentifierTable.cpp Modified: cfe/trunk/include/clang/Basic/IdentifierTable.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/IdentifierTable.h?rev=333657&r1=333656&r2=333657&view=diff == --- cfe/trunk/include/clang/Basic/IdentifierTable.h (original) +++ cfe/trunk/include/clang/Basic/IdentifierTable.h Thu May 31 07:45:32 2018 @@ -763,6 +763,8 @@ public: /// Prints the full selector name (e.g. "foo:bar:"). void print(llvm::raw_ostream &OS) const; + void dump() const; + /// Derive the conventional family of this method. ObjCMethodFamily getMethodFamily() const { return getMethodFamilyImpl(*this); Modified: cfe/trunk/lib/Basic/IdentifierTable.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/IdentifierTable.cpp?rev=333657&r1=333656&r2=333657&view=diff == --- cfe/trunk/lib/Basic/IdentifierTable.cpp (original) +++ cfe/trunk/lib/Basic/IdentifierTable.cpp Thu May 31 07:45:32 2018 @@ -504,6 +504,8 @@ void Selector::print(llvm::raw_ostream & OS << getAsString(); } +LLVM_DUMP_METHOD void Selector::dump() const { print(llvm::errs()); } + /// Interpreting the given string using the normal CamelCase /// conventions, determine whether the given string starts with the /// given "word", which is assumed to end in a lowercase letter. ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45935: Add dump method for selectors
This revision was automatically updated to reflect the committed changes. Closed by commit rL333657: Add dump method for selectors (authored by hiraditya, committed by ). Changed prior to commit: https://reviews.llvm.org/D45935?vs=143493&id=149294#toc Repository: rL LLVM https://reviews.llvm.org/D45935 Files: cfe/trunk/include/clang/Basic/IdentifierTable.h cfe/trunk/lib/Basic/IdentifierTable.cpp Index: cfe/trunk/include/clang/Basic/IdentifierTable.h === --- cfe/trunk/include/clang/Basic/IdentifierTable.h +++ cfe/trunk/include/clang/Basic/IdentifierTable.h @@ -763,6 +763,8 @@ /// Prints the full selector name (e.g. "foo:bar:"). void print(llvm::raw_ostream &OS) const; + void dump() const; + /// Derive the conventional family of this method. ObjCMethodFamily getMethodFamily() const { return getMethodFamilyImpl(*this); Index: cfe/trunk/lib/Basic/IdentifierTable.cpp === --- cfe/trunk/lib/Basic/IdentifierTable.cpp +++ cfe/trunk/lib/Basic/IdentifierTable.cpp @@ -504,6 +504,8 @@ OS << getAsString(); } +LLVM_DUMP_METHOD void Selector::dump() const { print(llvm::errs()); } + /// Interpreting the given string using the normal CamelCase /// conventions, determine whether the given string starts with the /// given "word", which is assumed to end in a lowercase letter. Index: cfe/trunk/include/clang/Basic/IdentifierTable.h === --- cfe/trunk/include/clang/Basic/IdentifierTable.h +++ cfe/trunk/include/clang/Basic/IdentifierTable.h @@ -763,6 +763,8 @@ /// Prints the full selector name (e.g. "foo:bar:"). void print(llvm::raw_ostream &OS) const; + void dump() const; + /// Derive the conventional family of this method. ObjCMethodFamily getMethodFamily() const { return getMethodFamilyImpl(*this); Index: cfe/trunk/lib/Basic/IdentifierTable.cpp === --- cfe/trunk/lib/Basic/IdentifierTable.cpp +++ cfe/trunk/lib/Basic/IdentifierTable.cpp @@ -504,6 +504,8 @@ OS << getAsString(); } +LLVM_DUMP_METHOD void Selector::dump() const { print(llvm::errs()); } + /// Interpreting the given string using the normal CamelCase /// conventions, determine whether the given string starts with the /// given "word", which is assumed to end in a lowercase letter. ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47592: [AArch64] Corrected FP16 Intrinsic range checks in Clang + added Sema tests
LukeGeeson created this revision. LukeGeeson added a reviewer: SjoerdMeijer. Herald added a reviewer: javed.absar. Herald added a subscriber: kristof.beyls. This fixes the ranges for the vcvth family of FP16 intrinsics in the clang front end. Previously it was accepting incorrect ranges -Changed builtin range checking in SemaChecking -added tests SemaCheck changes - included in their own file since no similar one exists -modified existing tests to reflect new ranges https://reviews.llvm.org/D47592 Files: lib/Sema/SemaChecking.cpp test/CodeGen/aarch64-v8.2a-fp16-intrinsics.c test/Sema/aarch64-neon-fp16-ranges.c Index: test/Sema/aarch64-neon-fp16-ranges.c === --- /dev/null +++ test/Sema/aarch64-neon-fp16-ranges.c @@ -0,0 +1,64 @@ +// RUN: %clang_cc1 -triple arm64-linux-gnu -target-feature +neon -fallow-half-arguments-and-returns -target-feature +fullfp16 -ffreestanding -fsyntax-only -verify %s +// RUN: %clang_cc1 -triple aarch64-linux-gnu -target-feature +neon -fallow-half-arguments-and-returns -target-feature +fullfp16 -ffreestanding -fsyntax-only -verify %s + +#include +#include + +void test_vcvt_f16_16(int16_t a){ + vcvth_n_f16_s16(a, 1); + vcvth_n_f16_s16(a, 16); + vcvth_n_f16_s16(a, 0); // expected-error {{argument should be a value from 1 to 16}} + vcvth_n_f16_s16(a, 17); // expected-error {{argument should be a value from 1 to 16}} + + vcvth_n_f16_u16(a, 1); + vcvth_n_f16_u16(a, 16); + vcvth_n_f16_u16(a, 0); // expected-error {{argument should be a value from 1 to 16}} + vcvth_n_f16_u16(a, 17); // expected-error {{argument should be a value from 1 to 16}} +} + +void test_vcvt_f16_32(int32_t a){ + vcvth_n_f16_u32(a, 1); + vcvth_n_f16_u32(a, 16); + vcvth_n_f16_u32(a, 0); // expected-error {{argument should be a value from 1 to 16}} + vcvth_n_f16_u32(a, 17); // expected-error {{argument should be a value from 1 to 16}} + + vcvth_n_f16_s32(a, 1); + vcvth_n_f16_s32(a, 16); + vcvth_n_f16_s32(a, 0); // expected-error {{argument should be a value from 1 to 16}} + vcvth_n_f16_s32(a, 17); // expected-error {{argument should be a value from 1 to 16}} +} + +void test_vcvt_f16_64(int64_t a){ + vcvth_n_f16_s64(a, 1); + vcvth_n_f16_s64(a, 16); + vcvth_n_f16_s64(a, 0); // expected-error {{argument should be a value from 1 to 16}} + vcvth_n_f16_s64(a, 17); // expected-error {{argument should be a value from 1 to 16}} +} + + +void test_vcvt_su_f(int64_t a){ + vcvth_n_s16_f16(a, 1); + vcvth_n_s16_f16(a, 16); + vcvth_n_s16_f16(a, 0); // expected-error {{argument should be a value from 1 to 16}} + vcvth_n_s16_f16(a, 17); // expected-error {{argument should be a value from 1 to 16}} + + vcvth_n_s32_f16(a, 1); + vcvth_n_s32_f16(a, 16); + vcvth_n_s32_f16(a, 0); // expected-error {{argument should be a value from 1 to 16}} + vcvth_n_s32_f16(a, 17); // expected-error {{argument should be a value from 1 to 16}} + + vcvth_n_s64_f16(a, 1); + vcvth_n_s64_f16(a, 32); + vcvth_n_s64_f16(a, 0); // expected-error {{argument should be a value from 1 to 32}} + vcvth_n_s64_f16(a, 33); // expected-error {{argument should be a value from 1 to 32}} + + vcvth_n_u16_f16(a, 1); + vcvth_n_u16_f16(a, 16); + vcvth_n_u16_f16(a, 0); // expected-error {{argument should be a value from 1 to 16}} + vcvth_n_u16_f16(a, 17); // expected-error {{argument should be a value from 1 to 16}} + + vcvth_n_u32_f16(a, 1); + vcvth_n_u32_f16(a, 16); + vcvth_n_u32_f16(a, 0); // expected-error {{argument should be a value from 1 to 16}} + vcvth_n_u32_f16(a, 17); // expected-error {{argument should be a value from 1 to 16}} +} Index: test/CodeGen/aarch64-v8.2a-fp16-intrinsics.c === --- test/CodeGen/aarch64-v8.2a-fp16-intrinsics.c +++ test/CodeGen/aarch64-v8.2a-fp16-intrinsics.c @@ -486,90 +486,90 @@ // CHECK-LABEL: test_vcvth_n_f16_s16 // CHECK: [[SEXT:%.*]] = sext i16 %a to i32 -// CHECK: [[CVT:%.*]] = call half @llvm.aarch64.neon.vcvtfxs2fp.f16.i32(i32 [[SEXT]], i32 0) +// CHECK: [[CVT:%.*]] = call half @llvm.aarch64.neon.vcvtfxs2fp.f16.i32(i32 [[SEXT]], i32 1) // CHECK: ret half [[CVT]] float16_t test_vcvth_n_f16_s16(int16_t a) { - return vcvth_n_f16_s16(a, 0); + return vcvth_n_f16_s16(a, 1); } // CHECK-LABEL: test_vcvth_n_f16_s32 -// CHECK: [[CVT:%.*]] = call half @llvm.aarch64.neon.vcvtfxs2fp.f16.i32(i32 %a, i32 0) +// CHECK: [[CVT:%.*]] = call half @llvm.aarch64.neon.vcvtfxs2fp.f16.i32(i32 %a, i32 1) // CHECK: ret half [[CVT]] float16_t test_vcvth_n_f16_s32(int32_t a) { - return vcvth_n_f16_s32(a, 0); + return vcvth_n_f16_s32(a, 1); } // CHECK-LABEL: test_vcvth_n_f16_s64 -// CHECK: [[CVT:%.*]] = call half @llvm.aarch64.neon.vcvtfxs2fp.f16.i64(i64 %a, i32 0) +// CHECK: [[CVT:%.*]] = call half @llvm.aarch64.neon.vcvtfxs2fp.f16.i64(i64 %a, i32 1) // CHECK: ret half [[CVT]] float16_t test_vcvth_n_f16_s64(int64_t a) { - return vcvth_n_f16_s64(a, 0
[PATCH] D47586: Update NRVO logic to support early return (Attempt 2)
tzik updated this revision to Diff 149299. tzik added a comment. test Repository: rC Clang https://reviews.llvm.org/D47586 Files: include/clang/AST/Decl.h include/clang/Sema/Scope.h lib/Sema/Scope.cpp lib/Sema/SemaDecl.cpp lib/Sema/SemaExpr.cpp lib/Sema/SemaStmt.cpp lib/Sema/SemaTemplateInstantiateDecl.cpp lib/Serialization/ASTReaderDecl.cpp lib/Serialization/ASTWriterDecl.cpp test/CodeGenCXX/nrvo-noopt.cpp test/CodeGenCXX/nrvo.cpp test/SemaCXX/nrvo-ast.cpp Index: test/SemaCXX/nrvo-ast.cpp === --- /dev/null +++ test/SemaCXX/nrvo-ast.cpp @@ -0,0 +1,153 @@ +// RUN: %clang_cc1 -fcxx-exceptions -fsyntax-only -ast-dump -o - %s | FileCheck %s + +struct X { + X(); + X(const X&); + X(X&&); +}; + +// CHECK-LABEL: FunctionDecl {{.*}} test_00 +X test_00() { + // CHECK: VarDecl {{.*}} x {{.*}} nrvo + X x; + return x; +} + +// CHECK-LABEL: FunctionDecl {{.*}} test_01 +X test_01(bool b) { + // CHECK: VarDecl {{.*}} x {{.*}} nrvo + X x; + if (b) +return x; + return x; +} + +// CHECK-LABEL: FunctionDecl {{.*}} test_02 +X test_02(bool b) { + // CHECK-NOT: VarDecl {{.*}} x {{.*}} nrvo + X x; + // CHECK-NOT: VarDecl {{.*}} y {{.*}} nrvo + X y; + if (b) +return y; + return x; +} + +// CHECK-LABEL: FunctionDecl {{.*}} test_03 +X test_03(bool b) { + if (b) { +// CHECK: VarDecl {{.*}} y {{.*}} nrvo +X y; +return y; + } + // CHECK: VarDecl {{.*}} x {{.*}} nrvo + X x; + return x; +} + +extern "C" _Noreturn void exit(int) throw(); + +// CHECK-LABEL: FunctionDecl {{.*}} test_04 +X test_04(bool b) { + { +// CHECK: VarDecl {{.*}} x {{.*}} nrvo +X x; +if (b) + return x; + } + exit(1); +} + +void may_throw(); +// CHECK-LABEL: FunctionDecl {{.*}} test_05 +X test_05() { + try { +may_throw(); +return X(); + } catch (X x) { +// CHECK-NOT: VarDecl {{.*}} x {{.*}} nrvo +return x; + } +} + +// CHECK-LABEL: FunctionDecl {{.*}} test_06 +X test_06() { + // CHECK-NOT: VarDecl {{.*}} x {{.*}} nrvo + X x __attribute__((aligned(8))); + return x; +} + +// CHECK-LABEL: FunctionDecl {{.*}} test_07 +X test_07(bool b) { + if (b) { +// CHECK: VarDecl {{.*}} x {{.*}} nrvo +X x; +return x; + } + return X(); +} + +// CHECK-LABEL: FunctionDecl {{.*}} test_08 +X test_08(bool b) { + if (b) { +// CHECK: VarDecl {{.*}} x {{.*}} nrvo +X x; +return x; + } else { +// CHECK: VarDecl {{.*}} y {{.*}} nrvo +X y; +return y; + } +} + +template +struct Y { + Y(); + // CHECK-LABEL: CXXMethodDecl {{.*}} test_09 'Y ()' + // CHECK: VarDecl {{.*}} y 'Y' nrvo + + // CHECK-LABEL: CXXMethodDecl {{.*}} test_09 'Y ()' + // CHECK: VarDecl {{.*}} y 'Y' nrvo + static Y test_09() { +Y y; +return y; + } +}; + +struct Z { + Z(const X&); +}; + +// CHECK-LABEL: FunctionDecl {{.*}} test_10 'A ()' +// CHECK: VarDecl {{.*}} b 'B' nrvo + +// CHECK-LABEL: FunctionDecl {{.*}} test_10 'X ()' +// CHECK: VarDecl {{.*}} b {{.*}} nrvo + +// CHECK-LABEL: FunctionDecl {{.*}} test_10 'Z ()' +// CHECK-NOT: VarDecl {{.*}} b {{.*}} nrvo +template +A test_10() { + B b; + return b; +} + +// CHECK-LABEL: FunctionDecl {{.*}} test_11 'A (bool)' +// CHECK-NOT: VarDecl {{.*}} a {{.*}} nrvo + +// CHECK-LABEL: FunctionDecl {{.*}} test_11 'X (bool)' +// CHECK-NOT: VarDecl {{.*}} a {{.*}} nrvo +template +A test_11(bool b) { + A a; + if (b) +return A(); + return a; +} + +void instantiate() { + Y::test_09(); + test_10(); + test_10(); + test_11(true); +} Index: test/CodeGenCXX/nrvo.cpp === --- test/CodeGenCXX/nrvo.cpp +++ test/CodeGenCXX/nrvo.cpp @@ -130,17 +130,13 @@ } // CHECK-LABEL: define void @_Z5test3b -X test3(bool B) { +X test3(bool B, X x) { // CHECK: tail call {{.*}} @_ZN1XC1Ev - // CHECK-NOT: call {{.*}} @_ZN1XC1ERKS_ - // CHECK: call {{.*}} @_ZN1XC1Ev - // CHECK: call {{.*}} @_ZN1XC1ERKS_ if (B) { X y; return y; } - // FIXME: we should NRVO this variable too. - X x; + // CHECK: tail call {{.*}} @_ZN1XC1ERKS_ return x; } @@ -191,21 +187,29 @@ } // CHECK-LABEL: define void @_Z5test7b +// CHECK-EH-LABEL: define void @_Z5test7b X test7(bool b) { // CHECK: tail call {{.*}} @_ZN1XC1Ev // CHECK-NEXT: ret + + // CHECK-EH: tail call {{.*}} @_ZN1XC1Ev + // CHECK-EH-NEXT: ret if (b) { X x; return x; } return X(); } // CHECK-LABEL: define void @_Z5test8b +// CHECK-EH-LABEL: define void @_Z5test8b X test8(bool b) { // CHECK: tail call {{.*}} @_ZN1XC1Ev // CHECK-NEXT: ret - if (b) { + + // CHECK-EH: tail call {{.*}} @_ZN1XC1Ev + // CHECK-EH-NEXT: ret +if (b) { X x; return x; } else { @@ -221,4 +225,37 @@ // CHECK-LABEL: define linkonce_odr void @_ZN1YIiE1fEv // CHECK: tail call {{.*}} @_ZN1YIiEC1Ev +// CHECK-LABEL: define void @_Z6test10b +X test10(bool B, X x) { + if (B) { +// CHECK: tail call {{.*}} @_ZN1XC1ER
[PATCH] D47578: Do not enforce absolute path argv0 in windows
amccarth added a comment. I was under the impression that some tools rely on the fact that arg[0] is always expanded to an absolute path. Does this work with lldb and its test suite? https://reviews.llvm.org/D47578 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46664: Fix null MSInheritanceAttr deref in CXXRecordDecl::getMSInheritanceModel()
adr26 updated this revision to Diff 149300. adr26 added a comment. Hi Reid, I've added testcases matching the issues I hit in `microsoft-abi-member-pointers.cpp` (no change to the rest of the fix). If you are happy with this, please feel free to push. I checked with MSVC, and as per the added testcases, it appears to always lock in the inheritance model as virutal when there's a member funtion pointer in a dependant base, irrespective of whether the most-derived class ends up using single/multiple/virtual inheritance. In fact if, for example, you annotate DerivedFunctor (in the test I've added) using an MS inheritance keyword - i.e. as "class __single_inheritance DerviceFunctor" - then MSVC complains that it's already fixed the representation to be virtual: C:\Temp\clang\clang>cl C:\workspaces\llvm\tools\clang\test\CodeGenCXX\microsoft-abi-member-pointers.cpp Microsoft (R) C/C++ Optimizing Compiler Version 19.14.26428.1 for x86 Copyright (C) Microsoft Corporation. All rights reserved. microsoft-abi-member-pointers.cpp C:\workspaces\llvm\tools\clang\test\CodeGenCXX\microsoft-abi-member-pointers.cpp(81): error C2286: pointers to members of 'pr37399::DerivedFunctor' representation is already set to virtual_inheritance - declaration ignored C:\workspaces\llvm\tools\clang\test\CodeGenCXX\microsoft-abi-member-pointers.cpp(27): note: see declaration of 'pr37399::DerivedFunctor' as such, these tests cover the cases I was hitting and appear to have behavioural parity with MSVC (as is the intention!). Thanks, Andrew R https://reviews.llvm.org/D46664 Files: include/clang/AST/DeclCXX.h lib/AST/MicrosoftMangle.cpp lib/AST/Type.cpp lib/CodeGen/MicrosoftCXXABI.cpp lib/Sema/SemaType.cpp test/CodeGenCXX/microsoft-abi-member-pointers.cpp Index: test/CodeGenCXX/microsoft-abi-member-pointers.cpp === --- test/CodeGenCXX/microsoft-abi-member-pointers.cpp +++ test/CodeGenCXX/microsoft-abi-member-pointers.cpp @@ -3,6 +3,124 @@ // RUN: %clang_cc1 -std=c++11 -Wno-uninitialized -fno-rtti -emit-llvm %s -o - -triple=i386-pc-win32 -DINCOMPLETE_VIRTUAL -fms-extensions -verify // RUN: %clang_cc1 -std=c++11 -Wno-uninitialized -fno-rtti -emit-llvm %s -o - -triple=i386-pc-win32 -DINCOMPLETE_VIRTUAL -DMEMFUN -fms-extensions -verify +namespace pr37399 { +template +struct Functor { + void (T::*PtrToMemberFunction)(); +}; +// CHECK-DAG: %"struct.pr37399::Functor" = type { i8* } + +template +class SimpleDerivedFunctor; +template +class SimpleDerivedFunctor : public Functor> {}; +// CHECK-DAG: %"class.pr37399::SimpleDerivedFunctor" = type { %"struct.pr37399::Functor" } + +SimpleDerivedFunctor SimpleFunctor; +// CHECK-DAG: @"?SimpleFunctor@pr37399@@3V?$SimpleDerivedFunctor@X@1@A" = dso_local global %"class.pr37399::SimpleDerivedFunctor" zeroinitializer, align 4 + +short Global = 0; +template +class DerivedFunctor; +template +class DerivedFunctor +: public Functor> { +public: + void Foo() { +Global = 42; + } +}; + +class MultipleBase { +public: + MultipleBase() : Value() {} + short Value; +}; +// CHECK-DAG: %"class.pr37399::MultipleBase" = type { i16 } + +template +class MultiplyDerivedFunctor; +template +class MultiplyDerivedFunctor +: public Functor>, + public MultipleBase { +public: + void Foo() { +MultipleBase::Value = 42*2; + } +}; + +class VirtualBase { +public: + VirtualBase() : Value() {} + short Value; +}; +// CHECK-DAG: %"class.pr37399::VirtualBase" = type { i16 } + +template +class VirtBaseFunctor +: public Functor, + public virtual VirtualBase{}; +template +class VirtuallyDerivedFunctor; +template +class VirtuallyDerivedFunctor +: public VirtBaseFunctor>, + public virtual VirtualBase { +public: + void Foo() { +VirtualBase::Value = 42*3; + } +}; +} // namespace pr37399 + +pr37399::DerivedFunctor BFunctor; +// CHECK-DAG: @"?BFunctor@@3V?$DerivedFunctor@H@pr37399@@A" = dso_local global %"[[BFUNCTOR:class.pr37399::DerivedFunctor(\.[0-9]+)?]]" zeroinitializer, align 8 +// CHECK-DAG: %"[[BFUNCTOR]]" = type { %"[[BFUNCTORBASE:struct.pr37399::Functor(\.[0-9]+)?]]" } +// CHECK-DAG: %"[[BFUNCTORBASE]]" = type { { i8*, i32, i32, i32 } } +pr37399::DerivedFunctor AFunctor; +// CHECK-DAG: @"?AFunctor@@3V?$DerivedFunctor@X@pr37399@@A" = dso_local global %"[[AFUNCTOR:class.pr37399::DerivedFunctor(\.[0-9]+)?]]" zeroinitializer, align 8 +// CHECK-DAG: %"[[AFUNCTOR]]" = type { %"[[AFUNCTORBASE:struct.pr37399::Functor(\.[0-9]+)?]]" } +// CHECK-DAG: %"[[AFUNCTORBASE]]" = type { { i8*, i32, i32, i32 } } + +pr37399::MultiplyDerivedFunctor DFunctor; +// CHECK-DAG: @"?DFunctor@@3V?$MultiplyDerivedFunctor@H@pr37399@@A" = dso_local global %"[[DFUNCTOR:class.pr37399::MultiplyDerivedFunctor(\.[0-9]+)?]]" zeroinitializer, align 8 +// CHECK-DAG: %"[[DFUNCTOR]]" = type { %"[[DFUNCTORBASE:struct.pr37399::Functor(\.[0-9]+)?]]", %"class.pr37399::MultipleBase", [6 x i8]
[PATCH] D47578: Do not enforce absolute path argv0 in windows
takuto.ikuta updated this revision to Diff 149304. takuto.ikuta edited the summary of this revision. https://reviews.llvm.org/D47578 Files: llvm/lib/Support/Windows/Process.inc Index: llvm/lib/Support/Windows/Process.inc === --- llvm/lib/Support/Windows/Process.inc +++ llvm/lib/Support/Windows/Process.inc @@ -209,9 +209,7 @@ } static std::error_code ExpandShortFileName(const wchar_t *Arg, - SmallVectorImpl &Args, - BumpPtrAllocator &Alloc) { - SmallVector LongPath; + SmallVectorImpl &LongPath) { DWORD Length = GetLongPathNameW(Arg, LongPath.data(), LongPath.capacity()); if (Length == 0) return mapWindowsError(GetLastError()); @@ -222,7 +220,51 @@ return mapWindowsError(ERROR_INSUFFICIENT_BUFFER); } LongPath.set_size(Length); - return ConvertAndPushArg(LongPath.data(), Args, Alloc); + return std::error_code(); +} + +static std::error_code GetLongArgv0Path(const wchar_t *Argv0, +SmallVectorImpl &Args, +BumpPtrAllocator &Alloc) { + // The first argument may contain just the name of the executable (e.g., + // "clang") rather than the full path, so swap it with the full path. + wchar_t ModuleName[MAX_PATH]; + int Length = ::GetModuleFileNameW(NULL, ModuleName, MAX_PATH); + SmallVector UTF16LongpathArgv0, UTF16Argv0; + + if (0 < Length && Length < MAX_PATH) +Argv0 = ModuleName; + + // If the first argument is a shortened (8.3) name (which is possible even + // if we got the module name), the driver will have trouble distinguishing it + // (e.g., clang.exe v. clang++.exe), so expand it now. + std::error_code ec = ExpandShortFileName(Argv0, UTF16LongpathArgv0); + if (ec) +return ec; + + // Replace filename in original argv0 with expanded filename. + // This may change argv0 like below, + // * clang -> clang.exe (just add extension) + // * CLANG_~1.EXE -> clang++.exe (extend shorname) + // This is for keeping relativeness of original argv0 path. + SmallVector UTF8LongPathArgv0; + ec = windows::UTF16ToUTF8(UTF16LongpathArgv0.data(), UTF16LongpathArgv0.size(), +UTF8LongPathArgv0); + if (ec) +return ec; + + SmallVector UTF8Argv0; + ec = windows::UTF16ToUTF8(Argv0, wcslen(Argv0), UTF8Argv0); + if (ec) +return ec; + + sys::path::remove_filename(UTF8Argv0); + sys::path::append(UTF8Argv0, sys::path::filename(UTF8LongPathArgv0.data())); + ec = windows::UTF8ToUTF16(UTF8Argv0.data(), UTF16Argv0); + if (ec) +return ec; + + return ConvertAndPushArg(UTF16Argv0.data(), Args, Alloc); } std::error_code @@ -235,19 +277,7 @@ return mapWindowsError(::GetLastError()); Args.reserve(ArgCount); - std::error_code ec; - - // The first argument may contain just the name of the executable (e.g., - // "clang") rather than the full path, so swap it with the full path. - wchar_t ModuleName[MAX_PATH]; - int Length = ::GetModuleFileNameW(NULL, ModuleName, MAX_PATH); - if (0 < Length && Length < MAX_PATH) -UnicodeCommandLine[0] = ModuleName; - - // If the first argument is a shortened (8.3) name (which is possible even - // if we got the module name), the driver will have trouble distinguishing it - // (e.g., clang.exe v. clang++.exe), so expand it now. - ec = ExpandShortFileName(UnicodeCommandLine[0], Args, Alloc); + std::error_code ec = GetLongArgv0Path(UnicodeCommandLine[0], Args, Alloc); for (int i = 1; i < ArgCount && !ec; ++i) { ec = WildcardExpand(UnicodeCommandLine[i], Args, Alloc); Index: llvm/lib/Support/Windows/Process.inc === --- llvm/lib/Support/Windows/Process.inc +++ llvm/lib/Support/Windows/Process.inc @@ -209,9 +209,7 @@ } static std::error_code ExpandShortFileName(const wchar_t *Arg, - SmallVectorImpl &Args, - BumpPtrAllocator &Alloc) { - SmallVector LongPath; + SmallVectorImpl &LongPath) { DWORD Length = GetLongPathNameW(Arg, LongPath.data(), LongPath.capacity()); if (Length == 0) return mapWindowsError(GetLastError()); @@ -222,7 +220,51 @@ return mapWindowsError(ERROR_INSUFFICIENT_BUFFER); } LongPath.set_size(Length); - return ConvertAndPushArg(LongPath.data(), Args, Alloc); + return std::error_code(); +} + +static std::error_code GetLongArgv0Path(const wchar_t *Argv0, +SmallVectorImpl &Args, +BumpPtrAllocator &Alloc) { + // The first argument may contain just the name of the executable (e.g., + // "clang") rather than the full path, so swap it with the full path. + wchar_t ModuleName[MAX_PATH]; + int Length = ::Get
[PATCH] D47578: Do not enforce absolute path argv0 in windows
takuto.ikuta added a comment. In https://reviews.llvm.org/D47578#1117540, @ruiu wrote: > Looks like this patch contains two unrelated changes. Please separate your > change from the lit config changes. First patch made on some wrong assumption, fixed and reverted test config change. In https://reviews.llvm.org/D47578#1117760, @amccarth wrote: > I was under the impression that some tools rely on the fact that arg[0] is > always expanded to an absolute path. Does this work with lldb and its test > suite? I tried, but there is no check-lldb target. Can I ask you what is the target name to run lldb test suite? https://reviews.llvm.org/D47578 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
leonardchan updated this revision to Diff 149305. leonardchan marked 6 inline comments as done. Repository: rC Clang https://reviews.llvm.org/D46084 Files: include/clang-c/Index.h include/clang/AST/ASTContext.h include/clang/AST/BuiltinTypes.def include/clang/Basic/DiagnosticCommonKinds.td include/clang/Basic/LangOptions.def include/clang/Basic/Specifiers.h include/clang/Basic/TargetInfo.h include/clang/Basic/TokenKinds.def include/clang/Driver/Options.td include/clang/Sema/DeclSpec.h include/clang/Serialization/ASTBitCodes.h lib/AST/ASTContext.cpp lib/AST/ExprConstant.cpp lib/AST/ItaniumMangle.cpp lib/AST/MicrosoftMangle.cpp lib/AST/NSAPI.cpp lib/AST/Type.cpp lib/AST/TypeLoc.cpp lib/Analysis/PrintfFormatString.cpp lib/Basic/TargetInfo.cpp lib/CodeGen/CGDebugInfo.cpp lib/CodeGen/CodeGenTypes.cpp lib/CodeGen/ItaniumCXXABI.cpp lib/Driver/ToolChains/Clang.cpp lib/Frontend/CompilerInvocation.cpp lib/Index/USRGeneration.cpp lib/Parse/ParseDecl.cpp lib/Sema/DeclSpec.cpp lib/Sema/SemaTemplateVariadic.cpp lib/Sema/SemaType.cpp lib/Serialization/ASTCommon.cpp lib/Serialization/ASTReader.cpp test/Frontend/fixed_point.c test/Frontend/fixed_point_bit_widths.c test/Frontend/fixed_point_errors.c test/Frontend/fixed_point_errors.cpp test/Frontend/fixed_point_not_enabled.c tools/libclang/CXType.cpp Index: tools/libclang/CXType.cpp === --- tools/libclang/CXType.cpp +++ tools/libclang/CXType.cpp @@ -53,6 +53,12 @@ BTCASE(Float); BTCASE(Double); BTCASE(LongDouble); +BTCASE(ShortAccum); +BTCASE(Accum); +BTCASE(LongAccum); +BTCASE(UShortAccum); +BTCASE(UAccum); +BTCASE(ULongAccum); BTCASE(Float16); BTCASE(Float128); BTCASE(NullPtr); @@ -546,6 +552,12 @@ TKIND(Float); TKIND(Double); TKIND(LongDouble); +TKIND(ShortAccum); +TKIND(Accum); +TKIND(LongAccum); +TKIND(UShortAccum); +TKIND(UAccum); +TKIND(ULongAccum); TKIND(Float16); TKIND(Float128); TKIND(NullPtr); Index: test/Frontend/fixed_point_not_enabled.c === --- /dev/null +++ test/Frontend/fixed_point_not_enabled.c @@ -0,0 +1,15 @@ +// RUN: %clang_cc1 -x c -verify %s + +// Primary fixed point types +signed short _Accum s_short_accum;// expected-error{{compile with '-ffixed-point' to enable fixed point types}} +signed _Accum s_accum;// expected-error{{compile with '-ffixed-point' to enable fixed point types}} +signed long _Accum s_long_accum; // expected-error{{compile with '-ffixed-point' to enable fixed point types}} +unsigned short _Accum u_short_accum; // expected-error{{compile with '-ffixed-point' to enable fixed point types}} +unsigned _Accum u_accum; // expected-error{{compile with '-ffixed-point' to enable fixed point types}} +unsigned long _Accum u_long_accum;// expected-error{{compile with '-ffixed-point' to enable fixed point types}} + +// Aliased fixed point types +short _Accum short_accum; // expected-error{{compile with '-ffixed-point' to enable fixed point types}} +_Accum accum; // expected-error{{compile with '-ffixed-point' to enable fixed point types}} + // expected-warning@-1{{type specifier missing, defaults to 'int'}} +long _Accum long_accum; // expected-error{{compile with '-ffixed-point' to enable fixed point types}} Index: test/Frontend/fixed_point_errors.cpp === --- /dev/null +++ test/Frontend/fixed_point_errors.cpp @@ -0,0 +1,5 @@ +// RUN: %clang_cc1 -x c++ -ffixed-point %s -verify + +// Name namgling is not provided for fixed point types in c++ + +_Accum accum; // expected-error{{unknown type name '_Accum'}} Index: test/Frontend/fixed_point_errors.c === --- /dev/null +++ test/Frontend/fixed_point_errors.c @@ -0,0 +1,27 @@ +// RUN: %clang_cc1 -verify -ffixed-point %s + +/* We do not yet support long long. No recommended bit widths are given for this + * size. */ + +long long _Accum longlong_accum; // expected-error{{'long long _Accum' is invalid}} +unsigned long long _Accum u_longlong_accum; // expected-error{{'long long _Accum' is invalid}} + +/* Although _Complex types work with floating point numbers, the extension + * provides no info for complex fixed point types. */ + +_Complex signed short _Accum cmplx_s_short_accum; // expected-error{{'_Complex _Accum' is invalid}} +_Complex signed _Accum cmplx_s_accum; // expected-error{{'_Complex _Accum' is invalid}} +_Complex signed long _Accum cmplx_s_long_accum; // expected-error{{'_Complex _Accum' is invalid}} +_Complex unsigned short _Accum cmplx_u_short_accum; // expected-error{
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
leonardchan added inline comments. Comment at: include/clang/Basic/TokenKinds.def:393 +// ISO/IEC JTC1 SC22 WG14 N1169 Extension +KEYWORD(_Accum , KEYALL) + rsmith wrote: > leonardchan wrote: > > jfb wrote: > > > ebevhan wrote: > > > > I believe that having KEYALL will enable the keyword even if > > > > -fno-fixed-point is given. Is this desired? It will mean that `_Accum` > > > > will not be a valid identifier in standard C regardless of the flag. > > > That seems fine: identifiers starting with underscore and a capital > > > letter already aren't valid identifiers in C and C++ because they're > > > reserved for the implementation. > > I think my test for `-fno-fixed-point` already catches this, but I did not > > notice until now the `KEYNOCXX` keyword until now. Using this instead > > allows for not having to check if the language is c++ since `_Accum` is no > > longer treated as a typename. The corresponding test checking fixed points > > in c++ has been updated to reflect this. > Just to make sure we're on the same page: it's OK to disable this feature for > C++ while you're working on it, but it needs to support C++ by the time > you're done. That's the goal. Just disabled in c++ for now with the end goal of getting c++ support. Right now I'm waiting for these types to be added in the next Itanium ABI, which I believe you responded to on Github. Afterwards I'll need to request Microsoft mangling. Comment at: lib/Basic/TargetInfo.cpp:45 + AccumWidth = AccumAlign = 32; + LongAccumWidth = LongAccumAlign = 64; SuitableAlign = 64; rsmith wrote: > jfb wrote: > > This seems weird because Targets which don't have these values for the > > non-Accum versions will have .e.g. `sizeof(short) != sizeof(short _Accum)`. > > Is there a point in ever having `_Accum` differ in size, width, and > > alignment from the underlying type? If not, can you set these values after > > the sub-target has specified its preferences? > I'm uncomfortable about opting all targets into this behavior with these > default values; this will result in an ABI break if later a target updates > these to the correct values. A per-target `AccumSupported` flag would help > substantially. But this is OK for the short term while you're still working > on the feature. > > We'll also need the target to inform us of the number of integer and > fractional bits for each such type. The integral and fractional bits will be set in the target and is available in a later patch. Comment at: lib/Basic/TargetInfo.cpp:45 + AccumWidth = AccumAlign = 32; + LongAccumWidth = LongAccumAlign = 64; SuitableAlign = 64; ebevhan wrote: > leonardchan wrote: > > rsmith wrote: > > > jfb wrote: > > > > This seems weird because Targets which don't have these values for the > > > > non-Accum versions will have .e.g. `sizeof(short) != sizeof(short > > > > _Accum)`. Is there a point in ever having `_Accum` differ in size, > > > > width, and alignment from the underlying type? If not, can you set > > > > these values after the sub-target has specified its preferences? > > > I'm uncomfortable about opting all targets into this behavior with these > > > default values; this will result in an ABI break if later a target > > > updates these to the correct values. A per-target `AccumSupported` flag > > > would help substantially. But this is OK for the short term while you're > > > still working on the feature. > > > > > > We'll also need the target to inform us of the number of integer and > > > fractional bits for each such type. > > The integral and fractional bits will be set in the target and is available > > in a later patch. > > We'll also need the target to inform us of the number of integer and > > fractional bits for each such type. > > I believe the only one that is needed is for the number of fractional bits; > the number of integer bits is implied by the difference between the type > width and fractional bits. I think I mention this in one of the other patches. > > You're right. I was stuck in the mindset that we would be providing an integral and fractional value. Repository: rC Clang https://reviews.llvm.org/D46084 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
leonardchan added inline comments. Comment at: lib/Basic/TargetInfo.cpp:45 + AccumWidth = AccumAlign = 32; + LongAccumWidth = LongAccumAlign = 64; SuitableAlign = 64; leonardchan wrote: > ebevhan wrote: > > leonardchan wrote: > > > rsmith wrote: > > > > jfb wrote: > > > > > This seems weird because Targets which don't have these values for > > > > > the non-Accum versions will have .e.g. `sizeof(short) != sizeof(short > > > > > _Accum)`. Is there a point in ever having `_Accum` differ in size, > > > > > width, and alignment from the underlying type? If not, can you set > > > > > these values after the sub-target has specified its preferences? > > > > I'm uncomfortable about opting all targets into this behavior with > > > > these default values; this will result in an ABI break if later a > > > > target updates these to the correct values. A per-target > > > > `AccumSupported` flag would help substantially. But this is OK for the > > > > short term while you're still working on the feature. > > > > > > > > We'll also need the target to inform us of the number of integer and > > > > fractional bits for each such type. > > > The integral and fractional bits will be set in the target and is > > > available in a later patch. > > > We'll also need the target to inform us of the number of integer and > > > fractional bits for each such type. > > > > I believe the only one that is needed is for the number of fractional bits; > > the number of integer bits is implied by the difference between the type > > width and fractional bits. I think I mention this in one of the other > > patches. > > > > > You're right. I was stuck in the mindset that we would be providing an > integral and fractional value. > The integral and fractional bits will be set in the target and is available > in a later patch. I mean just the fractional bits since the integral will just be the bit width minus fractional. Repository: rC Clang https://reviews.llvm.org/D46084 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36918: [Sema] Take into account the current context when checking the accessibility of a member function pointer
jkorous added a comment. I don't particularly like that between setting the DeclContext (SemaTemplateDeduction.cpp:3814) and actually using it (CheckAccess() in SemaAccess.cpp:1459) are some 20 stack frames but it looks like you already tried fixing this "locally" in your initial approach. I assume we can't get the appropriate DeclContext from Expr *OvlExpr in CheckAddressOfMemberAccess(), right? Repository: rC Clang https://reviews.llvm.org/D36918 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36918: [Sema] Take into account the current context when checking the accessibility of a member function pointer
ahatanak added a comment. It doesn't look like it's possible to get the context needed to do accessibility check (CXXMethodDecl 'method' for 'C::overloadedMethod' in the test case) from 'Expr *OvlExpr' in CheckAddressOfMemberAccess. It's possible to get the class in which the member is defined (class ''C' for C::overloadedMethod'), but we want the context in which the member is used (that is 'method'). Repository: rC Clang https://reviews.llvm.org/D36918 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47155: [analyzer] Improve simplifySVal performance.
This revision was automatically updated to reflect the committed changes. Closed by commit rL333670: [analyzer] Improve performance of the SVal simplification mechanism. (authored by dergachev, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D47155?vs=148684&id=149314#toc Repository: rL LLVM https://reviews.llvm.org/D47155 Files: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp cfe/trunk/test/Analysis/hangs.c Index: cfe/trunk/test/Analysis/hangs.c === --- cfe/trunk/test/Analysis/hangs.c +++ cfe/trunk/test/Analysis/hangs.c @@ -0,0 +1,30 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker core -verify %s + +// expected-no-diagnostics + +// Stuff that used to hang. + +int g(); + +int f(int y) { + return y + g(); +} + +int produce_a_very_large_symbol(int x) { + return f(f(f(f(f(f(f(f(f(f(f(f(f(f(f(f(f( + f(f(f(f(f(f(f(f(f(f(f(f(f(f(f(x; +} + +void produce_an_exponentially_exploding_symbol(int x, int y) { + x += y; y += x + g(); + x += y; y += x + g(); + x += y; y += x + g(); + x += y; y += x + g(); + x += y; y += x + g(); + x += y; y += x + g(); + x += y; y += x + g(); + x += y; y += x + g(); + x += y; y += x + g(); + x += y; y += x + g(); + x += y; y += x + g(); +} Index: cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp === --- cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp +++ cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp @@ -1222,6 +1222,10 @@ ProgramStateRef State; SValBuilder &SVB; +static bool isUnchanged(SymbolRef Sym, SVal Val) { + return Sym == Val.getAsSymbol(); +} + public: Simplifier(ProgramStateRef State) : State(State), SVB(State->getStateManager().getSValBuilder()) {} @@ -1231,15 +1235,16 @@ SVB.getKnownValue(State, nonloc::SymbolVal(S))) return Loc::isLocType(S->getType()) ? (SVal)SVB.makeIntLocVal(*I) : (SVal)SVB.makeIntVal(*I); - return Loc::isLocType(S->getType()) ? (SVal)SVB.makeLoc(S) - : nonloc::SymbolVal(S); + return SVB.makeSymbolVal(S); } // TODO: Support SymbolCast. Support IntSymExpr when/if we actually // start producing them. SVal VisitSymIntExpr(const SymIntExpr *S) { SVal LHS = Visit(S->getLHS()); + if (isUnchanged(S->getLHS(), LHS)) +return SVB.makeSymbolVal(S); SVal RHS; // By looking at the APSInt in the right-hand side of S, we cannot // figure out if it should be treated as a Loc or as a NonLoc. @@ -1264,6 +1269,8 @@ SVal VisitSymSymExpr(const SymSymExpr *S) { SVal LHS = Visit(S->getLHS()); SVal RHS = Visit(S->getRHS()); + if (isUnchanged(S->getLHS(), LHS) && isUnchanged(S->getRHS(), RHS)) +return SVB.makeSymbolVal(S); return SVB.evalBinOp(State, S->getOpcode(), LHS, RHS, S->getType()); } @@ -1274,13 +1281,20 @@ SVal VisitNonLocSymbolVal(nonloc::SymbolVal V) { // Simplification is much more costly than computing complexity. // For high complexity, it may be not worth it. - if (V.getSymbol()->computeComplexity() > 100) -return V; return Visit(V.getSymbol()); } SVal VisitSVal(SVal V) { return V; } }; - return Simplifier(State).Visit(V); + // A crude way of preventing this function from calling itself from evalBinOp. + static bool isReentering = false; + if (isReentering) +return V; + + isReentering = true; + SVal SimplifiedV = Simplifier(State).Visit(V); + isReentering = false; + + return SimplifiedV; } Index: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h === --- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h +++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h @@ -367,6 +367,15 @@ return loc::ConcreteInt(BasicVals.getValue(integer)); } + /// Make an SVal that represents the given symbol. This follows the convention + /// of representing Loc-type symbols (symbolic pointers and references) + /// as Loc values wrapping the symbol rather than as plain symbol values. + SVal makeSymbolVal(SymbolRef Sym) { +if (Loc::isLocType(Sym->getType())) + return makeLoc(Sym); +return nonloc::SymbolVal(Sym); + } + /// Return a memory region for the 'this' object reference. loc::MemRegionVal getCXXThis(const CXXMethodDecl *D, const StackFrameContext *SFC); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe
r333670 - [analyzer] Improve performance of the SVal simplification mechanism.
Author: dergachev Date: Thu May 31 10:22:38 2018 New Revision: 333670 URL: http://llvm.org/viewvc/llvm-project?rev=333670&view=rev Log: [analyzer] Improve performance of the SVal simplification mechanism. When neither LHS nor RHS of a binary operator expression can be simplified, return the original expression instead of re-evaluating the binary operator. Such re-evaluation was causing recusrive re-simplification which caused the algorithmic complexity to explode. Differential Revision: https://reviews.llvm.org/D47155 Added: cfe/trunk/test/Analysis/hangs.c Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h?rev=333670&r1=333669&r2=333670&view=diff == --- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h (original) +++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h Thu May 31 10:22:38 2018 @@ -367,6 +367,15 @@ public: return loc::ConcreteInt(BasicVals.getValue(integer)); } + /// Make an SVal that represents the given symbol. This follows the convention + /// of representing Loc-type symbols (symbolic pointers and references) + /// as Loc values wrapping the symbol rather than as plain symbol values. + SVal makeSymbolVal(SymbolRef Sym) { +if (Loc::isLocType(Sym->getType())) + return makeLoc(Sym); +return nonloc::SymbolVal(Sym); + } + /// Return a memory region for the 'this' object reference. loc::MemRegionVal getCXXThis(const CXXMethodDecl *D, const StackFrameContext *SFC); Modified: cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp?rev=333670&r1=333669&r2=333670&view=diff == --- cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp Thu May 31 10:22:38 2018 @@ -1222,6 +1222,10 @@ SVal SimpleSValBuilder::simplifySVal(Pro ProgramStateRef State; SValBuilder &SVB; +static bool isUnchanged(SymbolRef Sym, SVal Val) { + return Sym == Val.getAsSymbol(); +} + public: Simplifier(ProgramStateRef State) : State(State), SVB(State->getStateManager().getSValBuilder()) {} @@ -1231,8 +1235,7 @@ SVal SimpleSValBuilder::simplifySVal(Pro SVB.getKnownValue(State, nonloc::SymbolVal(S))) return Loc::isLocType(S->getType()) ? (SVal)SVB.makeIntLocVal(*I) : (SVal)SVB.makeIntVal(*I); - return Loc::isLocType(S->getType()) ? (SVal)SVB.makeLoc(S) - : nonloc::SymbolVal(S); + return SVB.makeSymbolVal(S); } // TODO: Support SymbolCast. Support IntSymExpr when/if we actually @@ -1240,6 +1243,8 @@ SVal SimpleSValBuilder::simplifySVal(Pro SVal VisitSymIntExpr(const SymIntExpr *S) { SVal LHS = Visit(S->getLHS()); + if (isUnchanged(S->getLHS(), LHS)) +return SVB.makeSymbolVal(S); SVal RHS; // By looking at the APSInt in the right-hand side of S, we cannot // figure out if it should be treated as a Loc or as a NonLoc. @@ -1264,6 +1269,8 @@ SVal SimpleSValBuilder::simplifySVal(Pro SVal VisitSymSymExpr(const SymSymExpr *S) { SVal LHS = Visit(S->getLHS()); SVal RHS = Visit(S->getRHS()); + if (isUnchanged(S->getLHS(), LHS) && isUnchanged(S->getRHS(), RHS)) +return SVB.makeSymbolVal(S); return SVB.evalBinOp(State, S->getOpcode(), LHS, RHS, S->getType()); } @@ -1274,13 +1281,20 @@ SVal SimpleSValBuilder::simplifySVal(Pro SVal VisitNonLocSymbolVal(nonloc::SymbolVal V) { // Simplification is much more costly than computing complexity. // For high complexity, it may be not worth it. - if (V.getSymbol()->computeComplexity() > 100) -return V; return Visit(V.getSymbol()); } SVal VisitSVal(SVal V) { return V; } }; - return Simplifier(State).Visit(V); + // A crude way of preventing this function from calling itself from evalBinOp. + static bool isReentering = false; + if (isReentering) +return V; + + isReentering = true; + SVal SimplifiedV = Simplifier(State).Visit(V); + isReentering = false; + + return SimplifiedV; } Added: cfe/trunk/test/Analysis/hangs.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/hangs.c?rev=333670&view=auto == --- cfe/trunk/test/Analysis/hangs.c (added) +++ cfe/trunk/test/Ana
[PATCH] D47578: Do not enforce absolute path argv0 in windows
rnk added a comment. I think this would be easy to unit test in llvm/unittests/Support/CommandLine.cpp. We'd just check that the filename is "SupportTests.exe" on Windows and the path is relative after calling this, I guess. Right? Look at how ProgramTest.cpp does this to get a reasonable argv0: sys::fs::getMainExecutable(TestMainArgv0, &ProgramTestStringArg1); In https://reviews.llvm.org/D47578#1117790, @takuto.ikuta wrote: > In https://reviews.llvm.org/D47578#1117760, @amccarth wrote: > > > I was under the impression that some tools rely on the fact that arg[0] is > > always expanded to an absolute path. Does this work with lldb and its test > > suite? > > > I tried, but there is no check-lldb target. Can I ask you what is the target > name to run lldb test suite? The LLDB test suite isn't in very good shape on Windows. It is complicated to set up and build, I don't want to block this fix on @takuto.ikuta setting up that build environment. This is a Windows-only change, and I believe it makes it more consistent with Linux, so as long as check-llvm, check-clang, and check-lld pass, this should be relatively safe. https://reviews.llvm.org/D47578 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47402: [analyzer] Improve simplifySVal performance further.
This revision was automatically updated to reflect the committed changes. Closed by commit rL333671: [analyzer] Improve performance of the SVal simplification mechanism further. (authored by dergachev, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D47402?vs=148695&id=149315#toc Repository: rL LLVM https://reviews.llvm.org/D47402 Files: cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp Index: cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp === --- cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp +++ cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp @@ -1222,6 +1222,12 @@ ProgramStateRef State; SValBuilder &SVB; +// Cache results for the lifetime of the Simplifier. Results change every +// time new constraints are added to the program state, which is the whole +// point of simplifying, and for that very reason it's pointless to maintain +// the same cache for the duration of the whole analysis. +llvm::DenseMap Cached; + static bool isUnchanged(SymbolRef Sym, SVal Val) { return Sym == Val.getAsSymbol(); } @@ -1242,9 +1248,16 @@ // start producing them. SVal VisitSymIntExpr(const SymIntExpr *S) { + auto I = Cached.find(S); + if (I != Cached.end()) +return I->second; + SVal LHS = Visit(S->getLHS()); - if (isUnchanged(S->getLHS(), LHS)) -return SVB.makeSymbolVal(S); + if (isUnchanged(S->getLHS(), LHS)) { +SVal V = SVB.makeSymbolVal(S); +Cached[S] = V; +return V; + } SVal RHS; // By looking at the APSInt in the right-hand side of S, we cannot // figure out if it should be treated as a Loc or as a NonLoc. @@ -1263,15 +1276,27 @@ } else { RHS = SVB.makeIntVal(S->getRHS()); } - return SVB.evalBinOp(State, S->getOpcode(), LHS, RHS, S->getType()); + + SVal V = SVB.evalBinOp(State, S->getOpcode(), LHS, RHS, S->getType()); + Cached[S] = V; + return V; } SVal VisitSymSymExpr(const SymSymExpr *S) { + auto I = Cached.find(S); + if (I != Cached.end()) +return I->second; + SVal LHS = Visit(S->getLHS()); SVal RHS = Visit(S->getRHS()); - if (isUnchanged(S->getLHS(), LHS) && isUnchanged(S->getRHS(), RHS)) -return SVB.makeSymbolVal(S); - return SVB.evalBinOp(State, S->getOpcode(), LHS, RHS, S->getType()); + if (isUnchanged(S->getLHS(), LHS) && isUnchanged(S->getRHS(), RHS)) { +SVal V = SVB.makeSymbolVal(S); +Cached[S] = V; +return V; + } + SVal V = SVB.evalBinOp(State, S->getOpcode(), LHS, RHS, S->getType()); + Cached[S] = V; + return V; } SVal VisitSymExpr(SymbolRef S) { return nonloc::SymbolVal(S); } Index: cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp === --- cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp +++ cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp @@ -1222,6 +1222,12 @@ ProgramStateRef State; SValBuilder &SVB; +// Cache results for the lifetime of the Simplifier. Results change every +// time new constraints are added to the program state, which is the whole +// point of simplifying, and for that very reason it's pointless to maintain +// the same cache for the duration of the whole analysis. +llvm::DenseMap Cached; + static bool isUnchanged(SymbolRef Sym, SVal Val) { return Sym == Val.getAsSymbol(); } @@ -1242,9 +1248,16 @@ // start producing them. SVal VisitSymIntExpr(const SymIntExpr *S) { + auto I = Cached.find(S); + if (I != Cached.end()) +return I->second; + SVal LHS = Visit(S->getLHS()); - if (isUnchanged(S->getLHS(), LHS)) -return SVB.makeSymbolVal(S); + if (isUnchanged(S->getLHS(), LHS)) { +SVal V = SVB.makeSymbolVal(S); +Cached[S] = V; +return V; + } SVal RHS; // By looking at the APSInt in the right-hand side of S, we cannot // figure out if it should be treated as a Loc or as a NonLoc. @@ -1263,15 +1276,27 @@ } else { RHS = SVB.makeIntVal(S->getRHS()); } - return SVB.evalBinOp(State, S->getOpcode(), LHS, RHS, S->getType()); + + SVal V = SVB.evalBinOp(State, S->getOpcode(), LHS, RHS, S->getType()); + Cached[S] = V; + return V; } SVal VisitSymSymExpr(const SymSymExpr *S) { + auto I = Cached.find(S); + if (I != Cached.end()) +return I->second; + SVal LHS = Visit(S->getLHS()); SVal RHS = Visit(S->getRHS()); - if (isUnchanged(S->getLHS(), LHS) && isUnchanged(S->getRHS(), RHS)) -return SVB.makeSymbolVal(S); - return SVB.evalBinOp(State, S->getOpcode(), LHS, RHS, S->getType()); + if (i
r333671 - [analyzer] Improve performance of the SVal simplification mechanism further.
Author: dergachev Date: Thu May 31 10:27:28 2018 New Revision: 333671 URL: http://llvm.org/viewvc/llvm-project?rev=333671&view=rev Log: [analyzer] Improve performance of the SVal simplification mechanism further. Memoize simplification so that we didn't need to simplify the same symbolic expression twice within the same program state. Gives ~25% performance boost on the artificial test in test/Analysis/hangs.c. Differential Revision: https://reviews.llvm.org/D47402 Modified: cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp Modified: cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp?rev=333671&r1=333670&r2=333671&view=diff == --- cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp Thu May 31 10:27:28 2018 @@ -1222,6 +1222,12 @@ SVal SimpleSValBuilder::simplifySVal(Pro ProgramStateRef State; SValBuilder &SVB; +// Cache results for the lifetime of the Simplifier. Results change every +// time new constraints are added to the program state, which is the whole +// point of simplifying, and for that very reason it's pointless to maintain +// the same cache for the duration of the whole analysis. +llvm::DenseMap Cached; + static bool isUnchanged(SymbolRef Sym, SVal Val) { return Sym == Val.getAsSymbol(); } @@ -1242,9 +1248,16 @@ SVal SimpleSValBuilder::simplifySVal(Pro // start producing them. SVal VisitSymIntExpr(const SymIntExpr *S) { + auto I = Cached.find(S); + if (I != Cached.end()) +return I->second; + SVal LHS = Visit(S->getLHS()); - if (isUnchanged(S->getLHS(), LHS)) -return SVB.makeSymbolVal(S); + if (isUnchanged(S->getLHS(), LHS)) { +SVal V = SVB.makeSymbolVal(S); +Cached[S] = V; +return V; + } SVal RHS; // By looking at the APSInt in the right-hand side of S, we cannot // figure out if it should be treated as a Loc or as a NonLoc. @@ -1263,15 +1276,27 @@ SVal SimpleSValBuilder::simplifySVal(Pro } else { RHS = SVB.makeIntVal(S->getRHS()); } - return SVB.evalBinOp(State, S->getOpcode(), LHS, RHS, S->getType()); + + SVal V = SVB.evalBinOp(State, S->getOpcode(), LHS, RHS, S->getType()); + Cached[S] = V; + return V; } SVal VisitSymSymExpr(const SymSymExpr *S) { + auto I = Cached.find(S); + if (I != Cached.end()) +return I->second; + SVal LHS = Visit(S->getLHS()); SVal RHS = Visit(S->getRHS()); - if (isUnchanged(S->getLHS(), LHS) && isUnchanged(S->getRHS(), RHS)) -return SVB.makeSymbolVal(S); - return SVB.evalBinOp(State, S->getOpcode(), LHS, RHS, S->getType()); + if (isUnchanged(S->getLHS(), LHS) && isUnchanged(S->getRHS(), RHS)) { +SVal V = SVB.makeSymbolVal(S); +Cached[S] = V; +return V; + } + SVal V = SVB.evalBinOp(State, S->getOpcode(), LHS, RHS, S->getType()); + Cached[S] = V; + return V; } SVal VisitSymExpr(SymbolRef S) { return nonloc::SymbolVal(S); } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47578: Do not enforce absolute path argv0 in windows
amccarth added a comment. In https://reviews.llvm.org/D47578#1117874, @rnk wrote: > The LLDB test suite isn't in very good shape on Windows. It is complicated to > set up and build, I don't want to block this fix on @takuto.ikuta setting up > that build environment. This is a Windows-only change, and I believe it makes > it more consistent with Linux, so as long as check-llvm, check-clang, and > check-lld pass, this should be relatively safe. OK, I'll take rnk's word for that. https://reviews.llvm.org/D47578 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r333673 - PrintEscapedString -> printEscapedString
Author: jdevlieghere Date: Thu May 31 10:36:31 2018 New Revision: 333673 URL: http://llvm.org/viewvc/llvm-project?rev=333673&view=rev Log: PrintEscapedString -> printEscapedString Update PrintEscapedString after renaming it in ADT. Modified: clang-tools-extra/trunk/clangd/Protocol.cpp Modified: clang-tools-extra/trunk/clangd/Protocol.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Protocol.cpp?rev=333673&r1=333672&r2=333673&view=diff == --- clang-tools-extra/trunk/clangd/Protocol.cpp (original) +++ clang-tools-extra/trunk/clangd/Protocol.cpp Thu May 31 10:36:31 2018 @@ -137,7 +137,7 @@ json::Expr toJSON(const TextEdit &P) { llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const TextEdit &TE) { OS << TE.range << " => \""; - PrintEscapedString(TE.newText, OS); + printEscapedString(TE.newText, OS); return OS << '"'; } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47557: Filesystem tests: un-confuse write time
EricWF added a comment. So the reason there are `atime` tests at all is due to the implementation details of `__last_write_time` here. Essentially we have to do extra work trying to maintain the access time. I'm OK with this patches rename. `first` and `second` were certainly not clear. That being said, I'm not sure we should be removing the access time tests, they were intentional. Repository: rCXX libc++ https://reviews.llvm.org/D47557 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47157: Warning for framework headers using double quote includes
bruno updated this revision to Diff 149313. bruno added a comment. Updated the patch after Duncan and Aaron reviews. I actually went a bit more aggressive with the fixits, since I realized the conditions for the warning are already strict enough and we should take the chance to be more clear. For the attached testcase, the output now is: ./A.framework/Headers/A.h:2:10: warning: double-quoted include "A0.h" in framework header, expected angle-bracketed include instead #include "A0.h" ^~ ./A.framework/Headers/A.h:3:10: warning: double-quoted include "B.h" in framework header, expected angle-bracketed include instead #include "B.h" ^ https://reviews.llvm.org/D47157 Files: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticLexKinds.td lib/Lex/HeaderSearch.cpp test/Modules/Inputs/double-quotes/A.framework/Headers/A.h test/Modules/Inputs/double-quotes/A.framework/Headers/A0.h test/Modules/Inputs/double-quotes/A.framework/Modules/module.modulemap test/Modules/Inputs/double-quotes/B.h test/Modules/Inputs/double-quotes/X.framework/Headers/X.h test/Modules/Inputs/double-quotes/X.framework/Modules/module.modulemap test/Modules/Inputs/double-quotes/a.hmap.json test/Modules/Inputs/double-quotes/flat-header-path/Z.h test/Modules/Inputs/double-quotes/flat-header-path/Z.modulemap test/Modules/Inputs/double-quotes/x.hmap.json test/Modules/Inputs/double-quotes/z.yaml test/Modules/double-quotes.m Index: test/Modules/double-quotes.m === --- /dev/null +++ test/Modules/double-quotes.m @@ -0,0 +1,38 @@ +// REQUIRES: shell + +// RUN: rm -rf %t +// RUN: mkdir %t + +// RUN: hmaptool write %S/Inputs/double-quotes/a.hmap.json %t/a.hmap +// RUN: hmaptool write %S/Inputs/double-quotes/x.hmap.json %t/x.hmap + +// RUN: sed -e "s:TEST_DIR:%S/Inputs/double-quotes:g" \ +// RUN: %S/Inputs/double-quotes/z.yaml > %t/z.yaml + +// The output with and without modules should be the same, without modules first. +// RUN: %clang_cc1 \ +// RUN: -I %t/x.hmap -iquote %t/a.hmap -ivfsoverlay %t/z.yaml \ +// RUN: -F%S/Inputs/double-quotes -I%S/Inputs/double-quotes \ +// RUN: -Wquoted-include-in-framework-header -fsyntax-only %s -verify + +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache \ +// RUN: -I %t/x.hmap -iquote %t/a.hmap -ivfsoverlay %t/z.yaml \ +// RUN: -F%S/Inputs/double-quotes -I%S/Inputs/double-quotes \ +// RUN: -Wquoted-include-in-framework-header -fsyntax-only %s \ +// RUN: 2>%t/stderr + +// The same warnings show up when modules is on but -verify doesn't get it +// because they only show up under the module A building context. +// RUN: FileCheck --input-file=%t/stderr %s +// CHECK: double-quoted include "A0.h" in framework header, expected angle-bracketed include instead +// CHECK: double-quoted include "B.h" in framework header, expected angle-bracketed include instead +// CHECK: double-quoted include "B.h" in framework header, expected angle-bracketed include instead + +#import "A.h" +#import + +int bar() { return foo(); } + +// expected-warning@Inputs/double-quotes/A.framework/Headers/A.h:2{{double-quoted include "A0.h" in framework header}} +// expected-warning@Inputs/double-quotes/A.framework/Headers/A.h:3{{double-quoted include "B.h" in framework header}} +// expected-warning@Inputs/double-quotes/flat-header-path/Z.h:1{{double-quoted include "B.h" in framework header}} Index: test/Modules/Inputs/double-quotes/z.yaml === --- /dev/null +++ test/Modules/Inputs/double-quotes/z.yaml @@ -0,0 +1,28 @@ +{ + 'version': 0, + 'case-sensitive': 'false', + 'roots': [ +{ + 'type': 'directory', + 'name': "TEST_DIR/Z.framework/Headers", + 'contents': [ +{ + 'type': 'file', + 'name': "Z.h", + 'external-contents': "TEST_DIR/flat-header-path/Z.h" +} + ] +}, +{ + 'type': 'directory', + 'name': "TEST_DIR/Z.framework/Modules", + 'contents': [ +{ + 'type': 'file', + 'name': "module.modulemap", + 'external-contents': "TEST_DIR/flat-header-path/Z.modulemap" +} + ] +} + ] +} Index: test/Modules/Inputs/double-quotes/x.hmap.json === --- /dev/null +++ test/Modules/Inputs/double-quotes/x.hmap.json @@ -0,0 +1,7 @@ + +{ + "mappings" : +{ + "X.h" : "X/X.h" +} +} Index: test/Modules/Inputs/double-quotes/flat-header-path/Z.modulemap === --- /dev/null +++ test/Modules/Inputs/double-quotes/flat-header-path/Z.modulemap @@ -0,0 +1,3 @@ +framework module Z { + header "Z.h" +} Index: test/Modules/Inputs/double-quotes/flat-header-path/Z.h ===
[PATCH] D45517: [analyzer] False positive refutation with Z3
mikhail.ramalho updated this revision to Diff 149317. mikhail.ramalho marked 6 inline comments as not done. mikhail.ramalho added a comment. - Simplified refutation process: it now collects all the constraints in a given path and, only when it reaches the root node, the refutation manager is created and the constraints are checked for reachability. All the optimizations were removed. - Moved RangedConstraintManager.h to include/ - Moved refutation check to be the first in the list of BugVisitors - Added dump method to Z3Solver (to print the formula) - Added more documentation/comments https://reviews.llvm.org/D45517 Files: include/clang/StaticAnalyzer/Core/AnalyzerOptions.h include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h lib/StaticAnalyzer/Core/AnalyzerOptions.cpp lib/StaticAnalyzer/Core/BugReporter.cpp lib/StaticAnalyzer/Core/BugReporterVisitors.cpp lib/StaticAnalyzer/Core/RangeConstraintManager.cpp lib/StaticAnalyzer/Core/RangedConstraintManager.cpp lib/StaticAnalyzer/Core/RangedConstraintManager.h lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp test/Analysis/z3-crosscheck.c Index: test/Analysis/z3-crosscheck.c === --- /dev/null +++ test/Analysis/z3-crosscheck.c @@ -0,0 +1,57 @@ +// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -DNO_CROSSCHECK -verify %s +// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-config crosscheck-with-z3=true -verify %s +// REQUIRES: z3 + +#ifndef NO_CROSSCHECK +// expected-no-diagnostics +#endif + +int foo(int x) +{ + int *z = 0; + if ((x & 1) && ((x & 1) ^ 1)) +#ifdef NO_CROSSCHECK + return *z; // expected-warning {{Dereference of null pointer (loaded from variable 'z')}} +#else + return *z; // no-warning +#endif + return 0; +} + +void g(int d); + +void f(int *a, int *b) { + int c = 5; + if ((a - b) == 0) +c = 0; + if (a != b) +#ifdef NO_CROSSCHECK +g(3 / c); // expected-warning {{Division by zero}} +#else +g(3 / c); // no-warning +#endif +} + +_Bool nondet_bool(); + +void h(int d) { + int x, y, k, z = 1; +#ifdef NO_CROSSCHECK + while (z < k) { // expected-warning {{The right operand of '<' is a garbage value}} +#else + // FIXME: Should warn about 'k' being a garbage value + while (z < k) { // no-warning +#endif +z = 2 * z; + } +} + +void i() { + _Bool c = nondet_bool(); + if (c) { +h(1); + } else { +h(2); + } +} + Index: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp === --- lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp +++ lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp @@ -10,6 +10,7 @@ #include "clang/Basic/TargetInfo.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h" #include "clang/StaticAnalyzer/Core/PathSensitive/SimpleConstraintManager.h" #include "clang/Config/config.h" @@ -880,6 +881,12 @@ /// Reset the solver and remove all constraints. void reset() { Z3_solver_reset(Z3Context::ZC, Solver); } + + void print(raw_ostream &OS) const { +OS << Z3_solver_to_string(Z3Context::ZC, Solver); + } + + LLVM_DUMP_METHOD void dump() const { print(llvm::errs()); } }; // end class Z3Solver void Z3ErrorHandler(Z3_context Context, Z3_error_code Error) { @@ -915,6 +922,17 @@ void print(ProgramStateRef St, raw_ostream &Out, const char *nl, const char *sep) override; + void reset() override { Solver.reset(); } + + bool isModelFeasible() override { +return Solver.check() != Z3_L_FALSE; + } + + /// Converts the ranged constraints of a set of symbols to SMT + /// + /// \param CR The set of constraints. + void addRangeConstraints(ConstraintRangeTy CR) override; + //===--===// // Implementation for interface from SimpleConstraintManager. //===--===// @@ -1235,6 +1253,42 @@ return State->set(CZ); } +void Z3ConstraintManager::addRangeConstraints(ConstraintRangeTy CR) { + for (const auto &I : CR) { +SymbolRef Sym = I.first; + +Z3Expr Constraints = Z3Expr::fromBoolean(false); + +for (const auto &Range : I.second) { + const llvm::APSInt &From = Range.From(); + const llvm::APSInt &To = Range.To(); + + assert((getAPSIntType(From) == getAPSIntType(To)) && + "Range values have different types!"); + QualType RangeTy = getAPSIntType(From); + // Skip ranges whose endpoints cannot be converted to APSInts with + // a valid APSIntType. + // FIXME: fi
[PATCH] D45517: [analyzer] False positive refutation with Z3
mikhail.ramalho added subscribers: dcoughlin, george.karpenkov, NoQ. mikhail.ramalho added a comment. Hi, > Just a bit of context and to have some expectation management regarding > this patch. The main purpose of this implementation was to back a thesis. > It was made under a very serious time pressure and the main goal was to be > able to measure on real world projects as soon as possible and in the > meantime to be flexible so we can measure multiple configurations (like > incremental solving). > > So the goal was a flexible proof of concept that is sensible to measure in > the shortest possible time. After the thesis was done, Reka started to work > an another GSoC project, so she had no time to review the code with the > requirements of upstreaming in mind. Nevertheless we found that sharing the > proof of concept could be useful for the community. So it is perfectly > reasonable if you disagree with some design decisions behind this patch, > because the requirements for the thesis (in the short time frame) was very > different from the requirements of upstreaming this work. In a different > context these decisions made perfect sense. Just want to comment here and give thanks again for the first version of the refutation code. It's being really helpful to develop the approach this code as a base; things would definitely be slower if I had to start it from scratch. Thanks! https://reviews.llvm.org/D45517 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47597: IRGen: Write .dwo files when -split-dwarf-file is used together with -fthinlto-index.
pcc created this revision. pcc added reviewers: tejohnson, dblaikie. Herald added subscribers: JDevlieghere, hiraditya, eraman, inglorion, mehdi_amini. https://reviews.llvm.org/D47597 Files: clang/lib/CodeGen/BackendUtil.cpp clang/test/CodeGen/thinlto-split-dwarf.c llvm/include/llvm/LTO/Config.h llvm/lib/LTO/LTOBackend.cpp Index: llvm/lib/LTO/LTOBackend.cpp === --- llvm/lib/LTO/LTOBackend.cpp +++ llvm/lib/LTO/LTOBackend.cpp @@ -291,14 +291,19 @@ return; std::unique_ptr DwoOut; + SmallString<1024> DwoFile(Conf.DwoPath); if (!Conf.DwoDir.empty()) { std::error_code EC; if (auto EC = llvm::sys::fs::create_directories(Conf.DwoDir)) report_fatal_error("Failed to create directory " + Conf.DwoDir + ": " + EC.message()); -SmallString<1024> DwoFile(Conf.DwoDir); +DwoFile = Conf.DwoDir; sys::path::append(DwoFile, std::to_string(Task) + ".dwo"); + } + + if (!DwoFile.empty()) { +std::error_code EC; TM->Options.MCOptions.SplitDwarfFile = DwoFile.str().str(); DwoOut = llvm::make_unique(DwoFile, EC, sys::fs::F_None); if (EC) Index: llvm/include/llvm/LTO/Config.h === --- llvm/include/llvm/LTO/Config.h +++ llvm/include/llvm/LTO/Config.h @@ -76,6 +76,11 @@ /// The directory to store .dwo files. std::string DwoDir; + /// The path to write a .dwo file to. This should generally only be used when + /// running an individual backend directly via thinBackend(), as otherwise + /// all .dwo files will be written to the same path. + std::string DwoPath; + /// Optimization remarks file path. std::string RemarksFilename = ""; Index: clang/test/CodeGen/thinlto-split-dwarf.c === --- /dev/null +++ clang/test/CodeGen/thinlto-split-dwarf.c @@ -0,0 +1,21 @@ +// REQUIRES: x86-registered-target + +// RUN: %clang_cc1 -debug-info-kind=limited -triple x86_64-unknown-linux-gnu \ +// RUN: -flto=thin -emit-llvm-bc \ +// RUN: -o %t.o %s + +// RUN: llvm-lto2 run -thinlto-distributed-indexes %t.o \ +// RUN: -o %t2.index \ +// RUN: -r=%t.o,main,px + +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu \ +// RUN: -emit-obj -fthinlto-index=%t.o.thinlto.bc \ +// RUN: -o %t.native.o -split-dwarf-file %t.native.dwo -x ir %t.o + +// RUN: llvm-readobj -sections %t.native.o | FileCheck --check-prefix=O %s +// RUN: llvm-readobj -sections %t.native.dwo | FileCheck --check-prefix=DWO %s + +// O-NOT: .dwo +// DWO: .dwo + +int main() {} Index: clang/lib/CodeGen/BackendUtil.cpp === --- clang/lib/CodeGen/BackendUtil.cpp +++ clang/lib/CodeGen/BackendUtil.cpp @@ -1161,6 +1161,7 @@ Conf.DebugPassManager = CGOpts.DebugPassManager; Conf.RemarksWithHotness = CGOpts.DiagnosticsWithHotness; Conf.RemarksFilename = CGOpts.OptRecordFile; + Conf.DwoPath = CGOpts.SplitDwarfFile; switch (Action) { case Backend_EmitNothing: Conf.PreCodeGenModuleHook = [](size_t Task, const Module &Mod) { Index: llvm/lib/LTO/LTOBackend.cpp === --- llvm/lib/LTO/LTOBackend.cpp +++ llvm/lib/LTO/LTOBackend.cpp @@ -291,14 +291,19 @@ return; std::unique_ptr DwoOut; + SmallString<1024> DwoFile(Conf.DwoPath); if (!Conf.DwoDir.empty()) { std::error_code EC; if (auto EC = llvm::sys::fs::create_directories(Conf.DwoDir)) report_fatal_error("Failed to create directory " + Conf.DwoDir + ": " + EC.message()); -SmallString<1024> DwoFile(Conf.DwoDir); +DwoFile = Conf.DwoDir; sys::path::append(DwoFile, std::to_string(Task) + ".dwo"); + } + + if (!DwoFile.empty()) { +std::error_code EC; TM->Options.MCOptions.SplitDwarfFile = DwoFile.str().str(); DwoOut = llvm::make_unique(DwoFile, EC, sys::fs::F_None); if (EC) Index: llvm/include/llvm/LTO/Config.h === --- llvm/include/llvm/LTO/Config.h +++ llvm/include/llvm/LTO/Config.h @@ -76,6 +76,11 @@ /// The directory to store .dwo files. std::string DwoDir; + /// The path to write a .dwo file to. This should generally only be used when + /// running an individual backend directly via thinBackend(), as otherwise + /// all .dwo files will be written to the same path. + std::string DwoPath; + /// Optimization remarks file path. std::string RemarksFilename = ""; Index: clang/test/CodeGen/thinlto-split-dwarf.c === --- /dev/null +++ clang/test/CodeGen/thinlto-split-dwarf.c @@ -0,0 +1,21 @@ +// REQUIRES: x86-registered-target + +// RUN: %clang_cc1 -debug-info-kind=limited -triple x86_64-unknown-linux-gnu \ +// RUN: -flto=thin -emit-llvm-bc \ +// RUN: -o %t.o %s + +// RUN: ll
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
rsmith added inline comments. Comment at: lib/Basic/TargetInfo.cpp:45 + AccumWidth = AccumAlign = 32; + LongAccumWidth = LongAccumAlign = 64; SuitableAlign = 64; leonardchan wrote: > leonardchan wrote: > > ebevhan wrote: > > > leonardchan wrote: > > > > rsmith wrote: > > > > > jfb wrote: > > > > > > This seems weird because Targets which don't have these values for > > > > > > the non-Accum versions will have .e.g. `sizeof(short) != > > > > > > sizeof(short _Accum)`. Is there a point in ever having `_Accum` > > > > > > differ in size, width, and alignment from the underlying type? If > > > > > > not, can you set these values after the sub-target has specified > > > > > > its preferences? > > > > > I'm uncomfortable about opting all targets into this behavior with > > > > > these default values; this will result in an ABI break if later a > > > > > target updates these to the correct values. A per-target > > > > > `AccumSupported` flag would help substantially. But this is OK for > > > > > the short term while you're still working on the feature. > > > > > > > > > > We'll also need the target to inform us of the number of integer and > > > > > fractional bits for each such type. > > > > The integral and fractional bits will be set in the target and is > > > > available in a later patch. > > > > We'll also need the target to inform us of the number of integer and > > > > fractional bits for each such type. > > > > > > I believe the only one that is needed is for the number of fractional > > > bits; the number of integer bits is implied by the difference between the > > > type width and fractional bits. I think I mention this in one of the > > > other patches. > > > > > > > > You're right. I was stuck in the mindset that we would be providing an > > integral and fractional value. > > The integral and fractional bits will be set in the target and is available > > in a later patch. > > I mean just the fractional bits since the integral will just be the bit width > minus fractional. You're assuming that all bits will be either integral or fractional bits (and in particular that there are no padding bits). I don't know if that's actually going to be true for all target ABIs, but I suppose it's OK to make this assumption until it's proven wrong by some actual target. Repository: rC Clang https://reviews.llvm.org/D46084 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47578: Do not enforce absolute path argv0 in windows
ruiu added inline comments. Comment at: llvm/lib/Support/Windows/Process.inc:226 + +static std::error_code GetLongArgv0Path(const wchar_t *Argv0, +SmallVectorImpl &Args, It looks like this function is a bit too complicated. I'd define a function that expands DOS-style 8.3 path and adds missing extension, and use that function. https://reviews.llvm.org/D47578 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47597: IRGen: Write .dwo files when -split-dwarf-file is used together with -fthinlto-index.
tejohnson accepted this revision. tejohnson added a comment. This revision is now accepted and ready to land. LGTM thanks! https://reviews.llvm.org/D47597 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r333677 - IRGen: Write .dwo files when -split-dwarf-file is used together with -fthinlto-index.
Author: pcc Date: Thu May 31 11:25:59 2018 New Revision: 333677 URL: http://llvm.org/viewvc/llvm-project?rev=333677&view=rev Log: IRGen: Write .dwo files when -split-dwarf-file is used together with -fthinlto-index. Differential Revision: https://reviews.llvm.org/D47597 Added: cfe/trunk/test/CodeGen/thinlto-split-dwarf.c Modified: cfe/trunk/lib/CodeGen/BackendUtil.cpp Modified: cfe/trunk/lib/CodeGen/BackendUtil.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/BackendUtil.cpp?rev=333677&r1=333676&r2=333677&view=diff == --- cfe/trunk/lib/CodeGen/BackendUtil.cpp (original) +++ cfe/trunk/lib/CodeGen/BackendUtil.cpp Thu May 31 11:25:59 2018 @@ -1161,6 +1161,7 @@ static void runThinLTOBackend(ModuleSumm Conf.DebugPassManager = CGOpts.DebugPassManager; Conf.RemarksWithHotness = CGOpts.DiagnosticsWithHotness; Conf.RemarksFilename = CGOpts.OptRecordFile; + Conf.DwoPath = CGOpts.SplitDwarfFile; switch (Action) { case Backend_EmitNothing: Conf.PreCodeGenModuleHook = [](size_t Task, const Module &Mod) { Added: cfe/trunk/test/CodeGen/thinlto-split-dwarf.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/thinlto-split-dwarf.c?rev=333677&view=auto == --- cfe/trunk/test/CodeGen/thinlto-split-dwarf.c (added) +++ cfe/trunk/test/CodeGen/thinlto-split-dwarf.c Thu May 31 11:25:59 2018 @@ -0,0 +1,21 @@ +// REQUIRES: x86-registered-target + +// RUN: %clang_cc1 -debug-info-kind=limited -triple x86_64-unknown-linux-gnu \ +// RUN: -flto=thin -emit-llvm-bc \ +// RUN: -o %t.o %s + +// RUN: llvm-lto2 run -thinlto-distributed-indexes %t.o \ +// RUN: -o %t2.index \ +// RUN: -r=%t.o,main,px + +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu \ +// RUN: -emit-obj -fthinlto-index=%t.o.thinlto.bc \ +// RUN: -o %t.native.o -split-dwarf-file %t.native.dwo -x ir %t.o + +// RUN: llvm-readobj -sections %t.native.o | FileCheck --check-prefix=O %s +// RUN: llvm-readobj -sections %t.native.dwo | FileCheck --check-prefix=DWO %s + +// O-NOT: .dwo +// DWO: .dwo + +int main() {} ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47597: IRGen: Write .dwo files when -split-dwarf-file is used together with -fthinlto-index.
This revision was automatically updated to reflect the committed changes. Closed by commit rL333677: IRGen: Write .dwo files when -split-dwarf-file is used together with -fthinlto… (authored by pcc, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D47597?vs=149323&id=149326#toc Repository: rL LLVM https://reviews.llvm.org/D47597 Files: cfe/trunk/lib/CodeGen/BackendUtil.cpp cfe/trunk/test/CodeGen/thinlto-split-dwarf.c llvm/trunk/include/llvm/LTO/Config.h llvm/trunk/lib/LTO/LTOBackend.cpp Index: llvm/trunk/lib/LTO/LTOBackend.cpp === --- llvm/trunk/lib/LTO/LTOBackend.cpp +++ llvm/trunk/lib/LTO/LTOBackend.cpp @@ -291,14 +291,19 @@ return; std::unique_ptr DwoOut; + SmallString<1024> DwoFile(Conf.DwoPath); if (!Conf.DwoDir.empty()) { std::error_code EC; if (auto EC = llvm::sys::fs::create_directories(Conf.DwoDir)) report_fatal_error("Failed to create directory " + Conf.DwoDir + ": " + EC.message()); -SmallString<1024> DwoFile(Conf.DwoDir); +DwoFile = Conf.DwoDir; sys::path::append(DwoFile, std::to_string(Task) + ".dwo"); + } + + if (!DwoFile.empty()) { +std::error_code EC; TM->Options.MCOptions.SplitDwarfFile = DwoFile.str().str(); DwoOut = llvm::make_unique(DwoFile, EC, sys::fs::F_None); if (EC) Index: llvm/trunk/include/llvm/LTO/Config.h === --- llvm/trunk/include/llvm/LTO/Config.h +++ llvm/trunk/include/llvm/LTO/Config.h @@ -76,6 +76,11 @@ /// The directory to store .dwo files. std::string DwoDir; + /// The path to write a .dwo file to. This should generally only be used when + /// running an individual backend directly via thinBackend(), as otherwise + /// all .dwo files will be written to the same path. + std::string DwoPath; + /// Optimization remarks file path. std::string RemarksFilename = ""; Index: cfe/trunk/lib/CodeGen/BackendUtil.cpp === --- cfe/trunk/lib/CodeGen/BackendUtil.cpp +++ cfe/trunk/lib/CodeGen/BackendUtil.cpp @@ -1161,6 +1161,7 @@ Conf.DebugPassManager = CGOpts.DebugPassManager; Conf.RemarksWithHotness = CGOpts.DiagnosticsWithHotness; Conf.RemarksFilename = CGOpts.OptRecordFile; + Conf.DwoPath = CGOpts.SplitDwarfFile; switch (Action) { case Backend_EmitNothing: Conf.PreCodeGenModuleHook = [](size_t Task, const Module &Mod) { Index: cfe/trunk/test/CodeGen/thinlto-split-dwarf.c === --- cfe/trunk/test/CodeGen/thinlto-split-dwarf.c +++ cfe/trunk/test/CodeGen/thinlto-split-dwarf.c @@ -0,0 +1,21 @@ +// REQUIRES: x86-registered-target + +// RUN: %clang_cc1 -debug-info-kind=limited -triple x86_64-unknown-linux-gnu \ +// RUN: -flto=thin -emit-llvm-bc \ +// RUN: -o %t.o %s + +// RUN: llvm-lto2 run -thinlto-distributed-indexes %t.o \ +// RUN: -o %t2.index \ +// RUN: -r=%t.o,main,px + +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu \ +// RUN: -emit-obj -fthinlto-index=%t.o.thinlto.bc \ +// RUN: -o %t.native.o -split-dwarf-file %t.native.dwo -x ir %t.o + +// RUN: llvm-readobj -sections %t.native.o | FileCheck --check-prefix=O %s +// RUN: llvm-readobj -sections %t.native.dwo | FileCheck --check-prefix=DWO %s + +// O-NOT: .dwo +// DWO: .dwo + +int main() {} Index: llvm/trunk/lib/LTO/LTOBackend.cpp === --- llvm/trunk/lib/LTO/LTOBackend.cpp +++ llvm/trunk/lib/LTO/LTOBackend.cpp @@ -291,14 +291,19 @@ return; std::unique_ptr DwoOut; + SmallString<1024> DwoFile(Conf.DwoPath); if (!Conf.DwoDir.empty()) { std::error_code EC; if (auto EC = llvm::sys::fs::create_directories(Conf.DwoDir)) report_fatal_error("Failed to create directory " + Conf.DwoDir + ": " + EC.message()); -SmallString<1024> DwoFile(Conf.DwoDir); +DwoFile = Conf.DwoDir; sys::path::append(DwoFile, std::to_string(Task) + ".dwo"); + } + + if (!DwoFile.empty()) { +std::error_code EC; TM->Options.MCOptions.SplitDwarfFile = DwoFile.str().str(); DwoOut = llvm::make_unique(DwoFile, EC, sys::fs::F_None); if (EC) Index: llvm/trunk/include/llvm/LTO/Config.h === --- llvm/trunk/include/llvm/LTO/Config.h +++ llvm/trunk/include/llvm/LTO/Config.h @@ -76,6 +76,11 @@ /// The directory to store .dwo files. std::string DwoDir; + /// The path to write a .dwo file to. This should generally only be used when + /// running an individual backend directly via thinBackend(), as otherwise + /// all .dwo files will be written to the same path. + std::string DwoPath; + /// Optimization remarks file path. std::string RemarksFilename = ""; Index: cfe/trunk/lib/Code
[PATCH] D47157: Warning for framework headers using double quote includes
dexonsmith added inline comments. Comment at: test/Modules/double-quotes.m:24-25 + +// The same warnings show up when modules is on but -verify doesn't get it +// because they only show up under the module A building context. +// RUN: FileCheck --input-file=%t/stderr %s Would using an explicit module build make this any easier? Comment at: test/Modules/double-quotes.m:27-29 +// CHECK: double-quoted include "A0.h" in framework header, expected angle-bracketed include instead +// CHECK: double-quoted include "B.h" in framework header, expected angle-bracketed include instead +// CHECK: double-quoted include "B.h" in framework header, expected angle-bracketed include instead When there's a fixit, you don't need to list it in the warning text (the fix-it itself is sufficient). I also feel like "quoted include" is as clear as "double-quoted include" (but more succinct). So I think these would be better as: > warning: quoted include "..." in framework header, expected angle-bracketed > include instead https://reviews.llvm.org/D47157 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r333679 - [analyzer] Annotate ProgramState update methods with LLVM_NODISCARD.
Author: dergachev Date: Thu May 31 11:30:41 2018 New Revision: 333679 URL: http://llvm.org/viewvc/llvm-project?rev=333679&view=rev Log: [analyzer] Annotate ProgramState update methods with LLVM_NODISCARD. Because our program states are immutable, methods like "add<>", "set<>", "bind" create a copy of the program state instead of mutating the existing state. If the updated state is discarded, it clearly indicates a bug. Such bugs are introduced frequently, hence the warn_unused_result annotation. Differential Revision: https://reviews.llvm.org/D47499 Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h?rev=333679&r1=333678&r2=333679&view=diff == --- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h (original) +++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h Thu May 31 11:30:41 2018 @@ -177,20 +177,20 @@ public: /// /// This returns a new state with the added constraint on \p cond. /// If no new state is feasible, NULL is returned. - ProgramStateRef assume(DefinedOrUnknownSVal cond, bool assumption) const; + LLVM_NODISCARD ProgramStateRef assume(DefinedOrUnknownSVal cond, +bool assumption) const; /// Assumes both "true" and "false" for \p cond, and returns both /// corresponding states (respectively). /// /// This is more efficient than calling assume() twice. Note that one (but not /// both) of the returned states may be NULL. - std::pair + LLVM_NODISCARD std::pair assume(DefinedOrUnknownSVal cond) const; - ProgramStateRef assumeInBound(DefinedOrUnknownSVal idx, - DefinedOrUnknownSVal upperBound, - bool assumption, - QualType IndexType = QualType()) const; + LLVM_NODISCARD ProgramStateRef + assumeInBound(DefinedOrUnknownSVal idx, DefinedOrUnknownSVal upperBound, +bool assumption, QualType IndexType = QualType()) const; /// Assumes that the value of \p Val is bounded with [\p From; \p To] /// (if \p assumption is "true") or it is fully out of this range @@ -198,17 +198,17 @@ public: /// /// This returns a new state with the added constraint on \p cond. /// If no new state is feasible, NULL is returned. - ProgramStateRef assumeInclusiveRange(DefinedOrUnknownSVal Val, - const llvm::APSInt &From, - const llvm::APSInt &To, - bool assumption) const; + LLVM_NODISCARD ProgramStateRef assumeInclusiveRange(DefinedOrUnknownSVal Val, + const llvm::APSInt &From, + const llvm::APSInt &To, + bool assumption) const; /// Assumes given range both "true" and "false" for \p Val, and returns both /// corresponding states (respectively). /// /// This is more efficient than calling assume() twice. Note that one (but not /// both) of the returned states may be NULL. - std::pair + LLVM_NODISCARD std::pair assumeInclusiveRange(DefinedOrUnknownSVal Val, const llvm::APSInt &From, const llvm::APSInt &To) const; @@ -232,15 +232,16 @@ public: /// Create a new state by binding the value 'V' to the statement 'S' in the /// state's environment. - ProgramStateRef BindExpr(const Stmt *S, const LocationContext *LCtx, - SVal V, bool Invalidate = true) const; + LLVM_NODISCARD ProgramStateRef BindExpr(const Stmt *S, + const LocationContext *LCtx, SVal V, + bool Invalidate = true) const; + + LLVM_NODISCARD ProgramStateRef bindLoc(Loc location, SVal V, + const LocationContext *LCtx, + bool notifyChanges = true) const; - ProgramStateRef bindLoc(Loc location, - SVal V, - const LocationContext *LCtx, - bool notifyChanges = true) const; - - ProgramStateRef bindLoc(SVal location, SVal V, const LocationContext *LCtx) const; + LLVM_NODISCARD ProgramStateRef bindLoc(SVal location, SVal V, + const LocationContext *LCtx) const; /// Initializes the region of memory represented by \p loc with an initial /// value. Once initialized, all values loaded from any sub-regions of that @@ -248,14 +249,15 @@ public: /// This method should not be used on regions tha
[PATCH] D47499: [analyzer] Annotate program state update methods with LLVM_NODISCARD.
This revision was automatically updated to reflect the committed changes. Closed by commit rL333679: [analyzer] Annotate ProgramState update methods with LLVM_NODISCARD. (authored by dergachev, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D47499?vs=148988&id=149328#toc Repository: rL LLVM https://reviews.llvm.org/D47499 Files: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h Index: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h === --- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h +++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h @@ -177,38 +177,38 @@ /// /// This returns a new state with the added constraint on \p cond. /// If no new state is feasible, NULL is returned. - ProgramStateRef assume(DefinedOrUnknownSVal cond, bool assumption) const; + LLVM_NODISCARD ProgramStateRef assume(DefinedOrUnknownSVal cond, +bool assumption) const; /// Assumes both "true" and "false" for \p cond, and returns both /// corresponding states (respectively). /// /// This is more efficient than calling assume() twice. Note that one (but not /// both) of the returned states may be NULL. - std::pair + LLVM_NODISCARD std::pair assume(DefinedOrUnknownSVal cond) const; - ProgramStateRef assumeInBound(DefinedOrUnknownSVal idx, - DefinedOrUnknownSVal upperBound, - bool assumption, - QualType IndexType = QualType()) const; + LLVM_NODISCARD ProgramStateRef + assumeInBound(DefinedOrUnknownSVal idx, DefinedOrUnknownSVal upperBound, +bool assumption, QualType IndexType = QualType()) const; /// Assumes that the value of \p Val is bounded with [\p From; \p To] /// (if \p assumption is "true") or it is fully out of this range /// (if \p assumption is "false"). /// /// This returns a new state with the added constraint on \p cond. /// If no new state is feasible, NULL is returned. - ProgramStateRef assumeInclusiveRange(DefinedOrUnknownSVal Val, - const llvm::APSInt &From, - const llvm::APSInt &To, - bool assumption) const; + LLVM_NODISCARD ProgramStateRef assumeInclusiveRange(DefinedOrUnknownSVal Val, + const llvm::APSInt &From, + const llvm::APSInt &To, + bool assumption) const; /// Assumes given range both "true" and "false" for \p Val, and returns both /// corresponding states (respectively). /// /// This is more efficient than calling assume() twice. Note that one (but not /// both) of the returned states may be NULL. - std::pair + LLVM_NODISCARD std::pair assumeInclusiveRange(DefinedOrUnknownSVal Val, const llvm::APSInt &From, const llvm::APSInt &To) const; @@ -232,30 +232,32 @@ /// Create a new state by binding the value 'V' to the statement 'S' in the /// state's environment. - ProgramStateRef BindExpr(const Stmt *S, const LocationContext *LCtx, - SVal V, bool Invalidate = true) const; + LLVM_NODISCARD ProgramStateRef BindExpr(const Stmt *S, + const LocationContext *LCtx, SVal V, + bool Invalidate = true) const; + + LLVM_NODISCARD ProgramStateRef bindLoc(Loc location, SVal V, + const LocationContext *LCtx, + bool notifyChanges = true) const; - ProgramStateRef bindLoc(Loc location, - SVal V, - const LocationContext *LCtx, - bool notifyChanges = true) const; - - ProgramStateRef bindLoc(SVal location, SVal V, const LocationContext *LCtx) const; + LLVM_NODISCARD ProgramStateRef bindLoc(SVal location, SVal V, + const LocationContext *LCtx) const; /// Initializes the region of memory represented by \p loc with an initial /// value. Once initialized, all values loaded from any sub-regions of that /// region will be equal to \p V, unless overwritten later by the program. /// This method should not be used on regions that are already initialized. /// If you need to indicate that memory contents have suddenly become unknown /// within a certain region of memory, consider invalidateRegions(). - ProgramStateRef bindDefaultInitial(SVal loc, SVal V, - const LocationContext *LCtx) const; + LLVM_NODISCARD ProgramStateRef
[PATCH] D47157: Warning for framework headers using double quote includes
aaron.ballman added inline comments. Comment at: test/Modules/double-quotes.m:27-29 +// CHECK: double-quoted include "A0.h" in framework header, expected angle-bracketed include instead +// CHECK: double-quoted include "B.h" in framework header, expected angle-bracketed include instead +// CHECK: double-quoted include "B.h" in framework header, expected angle-bracketed include instead dexonsmith wrote: > When there's a fixit, you don't need to list it in the warning text (the > fix-it itself is sufficient). I also feel like "quoted include" is as clear > as "double-quoted include" (but more succinct). So I think these would be > better as: > > > warning: quoted include "..." in framework header, expected angle-bracketed > > include instead > Some other lexer diagnostics use "double-quoted" when they want to distinguish with "angle-bracketed" (see `warn_pragma_include_alias_mismatch_angle` and `warn_pragma_include_alias_mismatch_quote` as examples). I don't have a strong opinion on what form we use, but I'd prefer for it to be consistent exposition. https://reviews.llvm.org/D47157 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r333680 - Fix null MSInheritanceAttr deref in CXXRecordDecl::getMSInheritanceModel()
Author: rnk Date: Thu May 31 11:42:29 2018 New Revision: 333680 URL: http://llvm.org/viewvc/llvm-project?rev=333680&view=rev Log: Fix null MSInheritanceAttr deref in CXXRecordDecl::getMSInheritanceModel() Ensure latest MPT decl has a MSInheritanceAttr when instantiating templates, to avoid null MSInheritanceAttr deref in CXXRecordDecl::getMSInheritanceModel(). See PR#37399 for repo / details. Patch by Andrew Rogers! Differential Revision: https://reviews.llvm.org/D46664 Modified: cfe/trunk/include/clang/AST/DeclCXX.h cfe/trunk/include/clang/AST/DeclTemplate.h cfe/trunk/lib/AST/MicrosoftMangle.cpp cfe/trunk/lib/AST/Type.cpp cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp cfe/trunk/lib/Sema/SemaType.cpp cfe/trunk/test/CodeGenCXX/microsoft-abi-member-pointers.cpp Modified: cfe/trunk/include/clang/AST/DeclCXX.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclCXX.h?rev=333680&r1=333679&r2=333680&view=diff == --- cfe/trunk/include/clang/AST/DeclCXX.h (original) +++ cfe/trunk/include/clang/AST/DeclCXX.h Thu May 31 11:42:29 2018 @@ -751,6 +751,21 @@ public: return const_cast(this)->getMostRecentDecl(); } + CXXRecordDecl *getMostRecentNonInjectedDecl() { +CXXRecordDecl *Recent = +static_cast(this)->getMostRecentDecl(); +while (Recent->isInjectedClassName()) { + // FIXME: Does injected class name need to be in the redeclarations chain? + assert(Recent->getPreviousDecl()); + Recent = Recent->getPreviousDecl(); +} +return Recent; + } + + const CXXRecordDecl *getMostRecentNonInjectedDecl() const { +return const_cast(this)->getMostRecentNonInjectedDecl(); + } + CXXRecordDecl *getDefinition() const { // We only need an update if we don't already know which // declaration is the definition. Modified: cfe/trunk/include/clang/AST/DeclTemplate.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclTemplate.h?rev=333680&r1=333679&r2=333680&view=diff == --- cfe/trunk/include/clang/AST/DeclTemplate.h (original) +++ cfe/trunk/include/clang/AST/DeclTemplate.h Thu May 31 11:42:29 2018 @@ -1720,14 +1720,8 @@ public: // it's not clear that we should override that, because the most recent // declaration as a CXXRecordDecl sometimes is the injected-class-name. ClassTemplateSpecializationDecl *getMostRecentDecl() { -CXXRecordDecl *Recent = static_cast( - this)->getMostRecentDecl(); -while (!isa(Recent)) { - // FIXME: Does injected class name need to be in the redeclarations chain? - assert(Recent->isInjectedClassName() && Recent->getPreviousDecl()); - Recent = Recent->getPreviousDecl(); -} -return cast(Recent); +return cast( +getMostRecentNonInjectedDecl()); } /// Retrieve the template that this specialization specializes. Modified: cfe/trunk/lib/AST/MicrosoftMangle.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/MicrosoftMangle.cpp?rev=333680&r1=333679&r2=333680&view=diff == --- cfe/trunk/lib/AST/MicrosoftMangle.cpp (original) +++ cfe/trunk/lib/AST/MicrosoftMangle.cpp Thu May 31 11:42:29 2018 @@ -1370,12 +1370,12 @@ void MicrosoftCXXNameMangler::mangleTemp const NamedDecl *ND = TA.getAsDecl(); if (isa(ND) || isa(ND)) { mangleMemberDataPointer( - cast(ND->getDeclContext())->getMostRecentDecl(), + cast(ND->getDeclContext())->getMostRecentNonInjectedDecl(), cast(ND)); } else if (const FunctionDecl *FD = dyn_cast(ND)) { const CXXMethodDecl *MD = dyn_cast(FD); if (MD && MD->isInstance()) { -mangleMemberFunctionPointer(MD->getParent()->getMostRecentDecl(), MD); + mangleMemberFunctionPointer(MD->getParent()->getMostRecentNonInjectedDecl(), MD); } else { Out << "$1?"; mangleName(FD); Modified: cfe/trunk/lib/AST/Type.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Type.cpp?rev=333680&r1=333679&r2=333680&view=diff == --- cfe/trunk/lib/AST/Type.cpp (original) +++ cfe/trunk/lib/AST/Type.cpp Thu May 31 11:42:29 2018 @@ -2040,7 +2040,7 @@ bool Type::isIncompleteType(NamedDecl ** return false; // The inheritance attribute might only be present on the most recent // CXXRecordDecl, use that one. -RD = RD->getMostRecentDecl(); +RD = RD->getMostRecentNonInjectedDecl(); // Nothing interesting to do if the inheritance attribute is already set. if (RD->hasAttr()) return false; @@ -3936,5 +3936,5 @@ QualType::DestructionKind QualType::isDe } CXXRecordDecl *MemberPointerType::getMostRecentCXXRecordDecl() const { - return getClass()->getAsCXX
[PATCH] D46664: Fix null MSInheritanceAttr deref in CXXRecordDecl::getMSInheritanceModel()
This revision was automatically updated to reflect the committed changes. Closed by commit rC333680: Fix null MSInheritanceAttr deref in CXXRecordDecl::getMSInheritanceModel() (authored by rnk, committed by ). Changed prior to commit: https://reviews.llvm.org/D46664?vs=149300&id=149329#toc Repository: rC Clang https://reviews.llvm.org/D46664 Files: include/clang/AST/DeclCXX.h include/clang/AST/DeclTemplate.h lib/AST/MicrosoftMangle.cpp lib/AST/Type.cpp lib/CodeGen/MicrosoftCXXABI.cpp lib/Sema/SemaType.cpp test/CodeGenCXX/microsoft-abi-member-pointers.cpp Index: lib/CodeGen/MicrosoftCXXABI.cpp === --- lib/CodeGen/MicrosoftCXXABI.cpp +++ lib/CodeGen/MicrosoftCXXABI.cpp @@ -2733,7 +2733,7 @@ assert(MD->isInstance() && "Member function must not be static!"); CharUnits NonVirtualBaseAdjustment = CharUnits::Zero(); - const CXXRecordDecl *RD = MD->getParent()->getMostRecentDecl(); + const CXXRecordDecl *RD = MD->getParent()->getMostRecentNonInjectedDecl(); CodeGenTypes &Types = CGM.getTypes(); unsigned VBTableIndex = 0; Index: lib/AST/MicrosoftMangle.cpp === --- lib/AST/MicrosoftMangle.cpp +++ lib/AST/MicrosoftMangle.cpp @@ -1370,12 +1370,12 @@ const NamedDecl *ND = TA.getAsDecl(); if (isa(ND) || isa(ND)) { mangleMemberDataPointer( - cast(ND->getDeclContext())->getMostRecentDecl(), + cast(ND->getDeclContext())->getMostRecentNonInjectedDecl(), cast(ND)); } else if (const FunctionDecl *FD = dyn_cast(ND)) { const CXXMethodDecl *MD = dyn_cast(FD); if (MD && MD->isInstance()) { -mangleMemberFunctionPointer(MD->getParent()->getMostRecentDecl(), MD); +mangleMemberFunctionPointer(MD->getParent()->getMostRecentNonInjectedDecl(), MD); } else { Out << "$1?"; mangleName(FD); Index: lib/AST/Type.cpp === --- lib/AST/Type.cpp +++ lib/AST/Type.cpp @@ -2040,7 +2040,7 @@ return false; // The inheritance attribute might only be present on the most recent // CXXRecordDecl, use that one. -RD = RD->getMostRecentDecl(); +RD = RD->getMostRecentNonInjectedDecl(); // Nothing interesting to do if the inheritance attribute is already set. if (RD->hasAttr()) return false; @@ -3936,5 +3936,5 @@ } CXXRecordDecl *MemberPointerType::getMostRecentCXXRecordDecl() const { - return getClass()->getAsCXXRecordDecl()->getMostRecentDecl(); + return getClass()->getAsCXXRecordDecl()->getMostRecentNonInjectedDecl(); } Index: lib/Sema/SemaType.cpp === --- lib/Sema/SemaType.cpp +++ lib/Sema/SemaType.cpp @@ -7544,7 +7544,7 @@ /// Locks in the inheritance model for the given class and all of its bases. static void assignInheritanceModel(Sema &S, CXXRecordDecl *RD) { - RD = RD->getMostRecentDecl(); + RD = RD->getMostRecentNonInjectedDecl(); if (!RD->hasAttr()) { MSInheritanceAttr::Spelling IM; Index: include/clang/AST/DeclCXX.h === --- include/clang/AST/DeclCXX.h +++ include/clang/AST/DeclCXX.h @@ -751,6 +751,21 @@ return const_cast(this)->getMostRecentDecl(); } + CXXRecordDecl *getMostRecentNonInjectedDecl() { +CXXRecordDecl *Recent = +static_cast(this)->getMostRecentDecl(); +while (Recent->isInjectedClassName()) { + // FIXME: Does injected class name need to be in the redeclarations chain? + assert(Recent->getPreviousDecl()); + Recent = Recent->getPreviousDecl(); +} +return Recent; + } + + const CXXRecordDecl *getMostRecentNonInjectedDecl() const { +return const_cast(this)->getMostRecentNonInjectedDecl(); + } + CXXRecordDecl *getDefinition() const { // We only need an update if we don't already know which // declaration is the definition. Index: include/clang/AST/DeclTemplate.h === --- include/clang/AST/DeclTemplate.h +++ include/clang/AST/DeclTemplate.h @@ -1720,14 +1720,8 @@ // it's not clear that we should override that, because the most recent // declaration as a CXXRecordDecl sometimes is the injected-class-name. ClassTemplateSpecializationDecl *getMostRecentDecl() { -CXXRecordDecl *Recent = static_cast( - this)->getMostRecentDecl(); -while (!isa(Recent)) { - // FIXME: Does injected class name need to be in the redeclarations chain? - assert(Recent->isInjectedClassName() && Recent->getPreviousDecl()); - Recent = Recent->getPreviousDecl(); -} -return cast(Recent); +return cast( +getMostRecentNonInjectedDecl()); } /// Retrieve the template that this specialization specializes. Index: test/CodeGenCXX/microsoft-abi-m
[PATCH] D46664: Fix null MSInheritanceAttr deref in CXXRecordDecl::getMSInheritanceModel()
rnk added a comment. Thanks for the patch, I committed it as r333680 with a minor modification. https://reviews.llvm.org/D46664 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45517: [analyzer] False positive refutation with Z3
rnkovacs added a comment. In https://reviews.llvm.org/D45517#1117898, @mikhail.ramalho wrote: > Just want to comment here and give thanks again for the first version of > the refutation code. It's being really helpful to develop the approach this > code as a base; things would definitely be slower if I had to start it from > scratch. @mikhail.ramalho Thanks for this note, it's very nice of you :) I'm glad if it saves a bit of time, but it's only a rough sketch, so please feel free to tailor it to your liking (and the reviewers' of course). https://reviews.llvm.org/D45517 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D47597: IRGen: Write .dwo files when -split-dwarf-file is used together with -fthinlto-index.
On Thu, May 31, 2018 at 11:20 AM Peter Collingbourne via Phabricator < revi...@reviews.llvm.org> wrote: > pcc created this revision. > pcc added reviewers: tejohnson, dblaikie. > Herald added subscribers: JDevlieghere, hiraditya, eraman, inglorion, > mehdi_amini. > > https://reviews.llvm.org/D47597 > > Files: > clang/lib/CodeGen/BackendUtil.cpp > clang/test/CodeGen/thinlto-split-dwarf.c > llvm/include/llvm/LTO/Config.h > llvm/lib/LTO/LTOBackend.cpp > > > Index: llvm/lib/LTO/LTOBackend.cpp > === > --- llvm/lib/LTO/LTOBackend.cpp > +++ llvm/lib/LTO/LTOBackend.cpp > @@ -291,14 +291,19 @@ > return; > >std::unique_ptr DwoOut; > + SmallString<1024> DwoFile(Conf.DwoPath); >if (!Conf.DwoDir.empty()) { > std::error_code EC; > if (auto EC = llvm::sys::fs::create_directories(Conf.DwoDir)) >report_fatal_error("Failed to create directory " + Conf.DwoDir + ": > " + > EC.message()); > > -SmallString<1024> DwoFile(Conf.DwoDir); > +DwoFile = Conf.DwoDir; > sys::path::append(DwoFile, std::to_string(Task) + ".dwo"); > + } > + > + if (!DwoFile.empty()) { > +std::error_code EC; > TM->Options.MCOptions.SplitDwarfFile = DwoFile.str().str(); > DwoOut = llvm::make_unique(DwoFile, EC, > sys::fs::F_None); > if (EC) > Index: llvm/include/llvm/LTO/Config.h > === > --- llvm/include/llvm/LTO/Config.h > +++ llvm/include/llvm/LTO/Config.h > @@ -76,6 +76,11 @@ >/// The directory to store .dwo files. >std::string DwoDir; > > + /// The path to write a .dwo file to. This should generally only be > used when > + /// running an individual backend directly via thinBackend(), as > otherwise > + /// all .dwo files will be written to the same path. > + std::string DwoPath; > + >/// Optimization remarks file path. >std::string RemarksFilename = ""; > > Index: clang/test/CodeGen/thinlto-split-dwarf.c > === > --- /dev/null > +++ clang/test/CodeGen/thinlto-split-dwarf.c > @@ -0,0 +1,21 @@ > +// REQUIRES: x86-registered-target > + > +// RUN: %clang_cc1 -debug-info-kind=limited -triple > x86_64-unknown-linux-gnu \ > +// RUN: -flto=thin -emit-llvm-bc \ > +// RUN: -o %t.o %s > + > +// RUN: llvm-lto2 run -thinlto-distributed-indexes %t.o \ > +// RUN: -o %t2.index \ > +// RUN: -r=%t.o,main,px > + > +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu \ > +// RUN: -emit-obj -fthinlto-index=%t.o.thinlto.bc \ > +// RUN: -o %t.native.o -split-dwarf-file %t.native.dwo -x ir %t.o > Can this be written in a single IR file yet (rather than frontend compiling, indexing, then backend compiling), now that Teresa's implemented some of the summary IR syntax? > + > +// RUN: llvm-readobj -sections %t.native.o | FileCheck --check-prefix=O %s > +// RUN: llvm-readobj -sections %t.native.dwo | FileCheck > --check-prefix=DWO %s > + > +// O-NOT: .dwo > +// DWO: .dwo > + > +int main() {} > Index: clang/lib/CodeGen/BackendUtil.cpp > === > --- clang/lib/CodeGen/BackendUtil.cpp > +++ clang/lib/CodeGen/BackendUtil.cpp > @@ -1161,6 +1161,7 @@ >Conf.DebugPassManager = CGOpts.DebugPassManager; >Conf.RemarksWithHotness = CGOpts.DiagnosticsWithHotness; >Conf.RemarksFilename = CGOpts.OptRecordFile; > + Conf.DwoPath = CGOpts.SplitDwarfFile; >switch (Action) { >case Backend_EmitNothing: > Conf.PreCodeGenModuleHook = [](size_t Task, const Module &Mod) { > > > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47600: [UBSan] DO NOT COMMIT: precise UBSan checks experiment
alekseyshl created this revision. clang part of the two part patch (LLVM + clang) LLVM part: https://reviews.llvm.org/D47599 Refer to https://reviews.llvm.org/D47599 for the detailed description. Repository: rC Clang https://reviews.llvm.org/D47600 Files: include/clang/Driver/Options.td include/clang/Frontend/CodeGenOptions.def lib/CodeGen/CGExpr.cpp lib/Driver/ToolChains/Clang.cpp lib/Frontend/CompilerInvocation.cpp Index: lib/Frontend/CompilerInvocation.cpp === --- lib/Frontend/CompilerInvocation.cpp +++ lib/Frontend/CompilerInvocation.cpp @@ -1118,6 +1118,10 @@ Opts.EmitVersionIdentMetadata = Args.hasFlag(OPT_Qy, OPT_Qn, true); + Opts.ExperimentalUseCondTrap = Args.hasFlag( + OPT_fexperimental_use_cond_trap, OPT_fno_experimental_use_cond_trap, + /* Default */ false); + return Success; } Index: lib/Driver/ToolChains/Clang.cpp === --- lib/Driver/ToolChains/Clang.cpp +++ lib/Driver/ToolChains/Clang.cpp @@ -4799,6 +4799,9 @@ CmdArgs.push_back("-fforce-enable-int128"); } + Args.AddLastArg(CmdArgs, options::OPT_fexperimental_use_cond_trap, + options::OPT_fno_experimental_use_cond_trap); + // Finally add the compile command to the compilation. if (Args.hasArg(options::OPT__SLASH_fallback) && Output.getType() == types::TY_Object && Index: lib/CodeGen/CGExpr.cpp === --- lib/CodeGen/CGExpr.cpp +++ lib/CodeGen/CGExpr.cpp @@ -3131,18 +3131,25 @@ void CodeGenFunction::EmitTrapCheck(llvm::Value *Checked) { llvm::BasicBlock *Cont = createBasicBlock("cont"); - // If we're optimizing, collapse all calls to trap down to just one per - // function to save on code size. - if (!CGM.getCodeGenOpts().OptimizationLevel || !TrapBB) { -TrapBB = createBasicBlock("trap"); + if (CGM.getCodeGenOpts().ExperimentalUseCondTrap) { +TrapBB = createBasicBlock("condtrap"); Builder.CreateCondBr(Checked, Cont, TrapBB); EmitBlock(TrapBB); -llvm::CallInst *TrapCall = EmitTrapCall(llvm::Intrinsic::trap); -TrapCall->setDoesNotReturn(); -TrapCall->setDoesNotThrow(); -Builder.CreateUnreachable(); +Builder.CreateCall(CGM.getIntrinsic(llvm::Intrinsic::condtrap)); } else { -Builder.CreateCondBr(Checked, Cont, TrapBB); +// If we're optimizing, collapse all calls to trap down to just one per +// function to save on code size. +if (!CGM.getCodeGenOpts().OptimizationLevel || !TrapBB) { + TrapBB = createBasicBlock("trap"); + Builder.CreateCondBr(Checked, Cont, TrapBB); + EmitBlock(TrapBB); + llvm::CallInst *TrapCall = EmitTrapCall(llvm::Intrinsic::trap); + TrapCall->setDoesNotReturn(); + TrapCall->setDoesNotThrow(); + Builder.CreateUnreachable(); +} else { + Builder.CreateCondBr(Checked, Cont, TrapBB); +} } EmitBlock(Cont); Index: include/clang/Frontend/CodeGenOptions.def === --- include/clang/Frontend/CodeGenOptions.def +++ include/clang/Frontend/CodeGenOptions.def @@ -332,6 +332,8 @@ /// Whether to embed source in DWARF debug line section. CODEGENOPT(EmbedSource, 1, 0) +CODEGENOPT(ExperimentalUseCondTrap, 1, 0) + #undef CODEGENOPT #undef ENUM_CODEGENOPT #undef VALUE_CODEGENOPT Index: include/clang/Driver/Options.td === --- include/clang/Driver/Options.td +++ include/clang/Driver/Options.td @@ -1102,6 +1102,10 @@ Alias, AliasArgs<["full"]>, HelpText<"Enable cf-protection in 'full' mode">; +def fexperimental_use_cond_trap : Flag<["-"], "fexperimental-use-cond-trap">, + Group, Flags<[CC1Option]>, + HelpText<"Enables an experimental conditional trap in LLVM.">; + def fxray_instrument : Flag<["-"], "fxray-instrument">, Group, Flags<[CC1Option]>, HelpText<"Generate XRay instrumentation sleds on function entry and exit">; @@ -1437,6 +1441,10 @@ def : Flag<["-"], "fno-aligned-new">, Alias; def faligned_new_EQ : Joined<["-"], "faligned-new=">; +def fno_experimental_use_cond_trap : Flag<["-"], "fno-experimental-use-cond-trap">, + Group, Flags<[CC1Option]>, + HelpText<"Disables an experimental conditional trap in LLVM.">; + def fobjc_legacy_dispatch : Flag<["-"], "fobjc-legacy-dispatch">, Group; def fobjc_new_property : Flag<["-"], "fobjc-new-property">, Group; def fobjc_infer_related_result_type : Flag<["-"], "fobjc-infer-related-result-type">, ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D47597: IRGen: Write .dwo files when -split-dwarf-file is used together with -fthinlto-index.
On Thu, May 31, 2018 at 11:54 AM, David Blaikie wrote: > > > On Thu, May 31, 2018 at 11:20 AM Peter Collingbourne via Phabricator < > revi...@reviews.llvm.org> wrote: > >> pcc created this revision. >> pcc added reviewers: tejohnson, dblaikie. >> Herald added subscribers: JDevlieghere, hiraditya, eraman, inglorion, >> mehdi_amini. >> >> https://reviews.llvm.org/D47597 >> >> Files: >> clang/lib/CodeGen/BackendUtil.cpp >> clang/test/CodeGen/thinlto-split-dwarf.c >> llvm/include/llvm/LTO/Config.h >> llvm/lib/LTO/LTOBackend.cpp >> >> >> Index: llvm/lib/LTO/LTOBackend.cpp >> === >> --- llvm/lib/LTO/LTOBackend.cpp >> +++ llvm/lib/LTO/LTOBackend.cpp >> @@ -291,14 +291,19 @@ >> return; >> >>std::unique_ptr DwoOut; >> + SmallString<1024> DwoFile(Conf.DwoPath); >>if (!Conf.DwoDir.empty()) { >> std::error_code EC; >> if (auto EC = llvm::sys::fs::create_directories(Conf.DwoDir)) >>report_fatal_error("Failed to create directory " + Conf.DwoDir + >> ": " + >> EC.message()); >> >> -SmallString<1024> DwoFile(Conf.DwoDir); >> +DwoFile = Conf.DwoDir; >> sys::path::append(DwoFile, std::to_string(Task) + ".dwo"); >> + } >> + >> + if (!DwoFile.empty()) { >> +std::error_code EC; >> TM->Options.MCOptions.SplitDwarfFile = DwoFile.str().str(); >> DwoOut = llvm::make_unique(DwoFile, EC, >> sys::fs::F_None); >> if (EC) >> Index: llvm/include/llvm/LTO/Config.h >> === >> --- llvm/include/llvm/LTO/Config.h >> +++ llvm/include/llvm/LTO/Config.h >> @@ -76,6 +76,11 @@ >>/// The directory to store .dwo files. >>std::string DwoDir; >> >> + /// The path to write a .dwo file to. This should generally only be >> used when >> + /// running an individual backend directly via thinBackend(), as >> otherwise >> + /// all .dwo files will be written to the same path. >> + std::string DwoPath; >> + >>/// Optimization remarks file path. >>std::string RemarksFilename = ""; >> >> Index: clang/test/CodeGen/thinlto-split-dwarf.c >> === >> --- /dev/null >> +++ clang/test/CodeGen/thinlto-split-dwarf.c >> @@ -0,0 +1,21 @@ >> +// REQUIRES: x86-registered-target >> + >> +// RUN: %clang_cc1 -debug-info-kind=limited -triple >> x86_64-unknown-linux-gnu \ >> +// RUN: -flto=thin -emit-llvm-bc \ >> +// RUN: -o %t.o %s >> + >> +// RUN: llvm-lto2 run -thinlto-distributed-indexes %t.o \ >> +// RUN: -o %t2.index \ >> +// RUN: -r=%t.o,main,px >> + >> +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu \ >> +// RUN: -emit-obj -fthinlto-index=%t.o.thinlto.bc \ >> +// RUN: -o %t.native.o -split-dwarf-file %t.native.dwo -x ir %t.o >> > > Can this be written in a single IR file yet (rather than frontend > compiling, indexing, then backend compiling), now that Teresa's implemented > some of the summary IR syntax? > I think the parsing part isn't implemented yet, so that wouldn't work. Besides, we would need two separate files anyway: the bitcode file (%t.o) and the combined summary (%t.o.thinlto.bc). Peter > > >> + >> +// RUN: llvm-readobj -sections %t.native.o | FileCheck --check-prefix=O >> %s >> +// RUN: llvm-readobj -sections %t.native.dwo | FileCheck >> --check-prefix=DWO %s >> + >> +// O-NOT: .dwo >> +// DWO: .dwo >> + >> +int main() {} >> Index: clang/lib/CodeGen/BackendUtil.cpp >> === >> --- clang/lib/CodeGen/BackendUtil.cpp >> +++ clang/lib/CodeGen/BackendUtil.cpp >> @@ -1161,6 +1161,7 @@ >>Conf.DebugPassManager = CGOpts.DebugPassManager; >>Conf.RemarksWithHotness = CGOpts.DiagnosticsWithHotness; >>Conf.RemarksFilename = CGOpts.OptRecordFile; >> + Conf.DwoPath = CGOpts.SplitDwarfFile; >>switch (Action) { >>case Backend_EmitNothing: >> Conf.PreCodeGenModuleHook = [](size_t Task, const Module &Mod) { >> >> >> -- -- Peter ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47445: [ASTImporter] Corrected diagnostic client handling in tests.
akyrtzi added a comment. Thanks for the explanation. Please do add documentation comments for the new method so people using ASTUnit in their own code have an idea when and why they would need to call this. Something like "if you intend to emit additional diagnostics after the ASTUnit is created [...]". Also consider making the naming more clear to match the intended purpose, like `enableEmittingAdditionalDiagnostics()` or something similar. Otherwise LGTM. Repository: rC Clang https://reviews.llvm.org/D47445 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46664: Fix null MSInheritanceAttr deref in CXXRecordDecl::getMSInheritanceModel()
adr26 added a comment. That's a nice change to avoid duplication in `ClassTemplateSpecializationDecl::getMostRecentDecl()`. Thanks for your help getting this out the door! Repository: rC Clang https://reviews.llvm.org/D46664 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47350: [analyzer] Track class member initializer constructors path-sensitively within their construction context.
NoQ updated this revision to Diff 149337. NoQ added a comment. Hmm, actually composition looks very pretty if you use the magic word "impl". https://reviews.llvm.org/D47350 Files: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h lib/StaticAnalyzer/Core/ExprEngine.cpp lib/StaticAnalyzer/Core/ExprEngineCXX.cpp test/Analysis/cxx17-mandatory-elision.cpp Index: test/Analysis/cxx17-mandatory-elision.cpp === --- test/Analysis/cxx17-mandatory-elision.cpp +++ test/Analysis/cxx17-mandatory-elision.cpp @@ -21,6 +21,37 @@ } // namespace variable_functional_cast_crash +namespace ctor_initializer { + +struct S { + int x, y, z; +}; + +struct T { + S s; + int w; + T(int w): s(), w(w) {} +}; + +class C { + T t; +public: + C() : t(T(4)) { +S s = {1, 2, 3}; +t.s = s; +// FIXME: Should be TRUE in C++11 as well. +clang_analyzer_eval(t.w == 4); +#if __cplusplus >= 201703L +// expected-warning@-2{{TRUE}} +#else +// expected-warning@-4{{UNKNOWN}} +#endif + } +}; + +} // namespace ctor_initializer + + namespace address_vector_tests { template struct AddressVector { Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp === --- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp +++ lib/StaticAnalyzer/Core/ExprEngineCXX.cpp @@ -153,6 +153,7 @@ QualType Ty = Field->getType(); FieldVal = makeZeroElementRegion(State, FieldVal, Ty, CallOpts.IsArrayCtorOrDtor); + State = addObjectUnderConstruction(State, Init, LCtx, FieldVal); return std::make_pair(State, FieldVal); } case ConstructionContext::NewAllocatedObjectKind: { @@ -272,35 +273,6 @@ State, loc::MemRegionVal(MRMgr.getCXXTempObjectRegion(E, LCtx))); } -const CXXConstructExpr * -ExprEngine::findDirectConstructorForCurrentCFGElement() { - // Go backward in the CFG to see if the previous element (ignoring - // destructors) was a CXXConstructExpr. If so, that constructor - // was constructed directly into an existing region. - // This process is essentially the inverse of that performed in - // findElementDirectlyInitializedByCurrentConstructor(). - if (currStmtIdx == 0) -return nullptr; - - const CFGBlock *B = getBuilderContext().getBlock(); - - unsigned int PreviousStmtIdx = currStmtIdx - 1; - CFGElement Previous = (*B)[PreviousStmtIdx]; - - while (Previous.getAs() && PreviousStmtIdx > 0) { ---PreviousStmtIdx; -Previous = (*B)[PreviousStmtIdx]; - } - - if (Optional PrevStmtElem = Previous.getAs()) { -if (auto *CtorExpr = dyn_cast(PrevStmtElem->getStmt())) { - return CtorExpr; -} - } - - return nullptr; -} - void ExprEngine::VisitCXXConstructExpr(const CXXConstructExpr *CE, ExplodedNode *Pred, ExplodedNodeSet &destNodes) { Index: lib/StaticAnalyzer/Core/ExprEngine.cpp === --- lib/StaticAnalyzer/Core/ExprEngine.cpp +++ lib/StaticAnalyzer/Core/ExprEngine.cpp @@ -109,7 +109,75 @@ // to the object's location, so that on every such statement the location // could have been retrieved. -typedef std::pair ConstructedObjectKey; +/// ConstructedObjectKey is used for being able to find the path-sensitive +/// memory region of a freshly constructed object while modeling the AST node +/// that syntactically represents the object that is being constructed. +/// Semantics of such nodes may sometimes require access to the region that's +/// not otherwise present in the program state, or to the very fact that +/// the construction context was present and contained references to these +/// AST nodes. +class ConstructedObjectKey { + typedef std::pair< + llvm::PointerUnion, + const LocationContext *> ConstructedObjectKeyImpl; + + ConstructedObjectKeyImpl Impl; + + const void *getAnyASTNodePtr() const { +if (const Stmt *S = getStmt()) + return S; +else + return getCXXCtorInitializer(); + } + +public: + ConstructedObjectKey( + llvm::PointerUnion P, + const LocationContext *LC) + : Impl(P, LC) { +// This is the full list of statements that require additional actions when +// encountered. This list may be expanded when new actions are implemented. +assert(getCXXCtorInitializer() || isa(getStmt()) || + isa(getStmt()) || isa(getStmt()) || + isa(getStmt())); + } + + const Stmt *getStmt() const { +return Impl.first.dyn_cast(); + } + + const CXXCtorInitializer *getCXXCtorInitializer() const { +return Impl.first.dyn_cast(); + } + + const LocationContext *getLocationContext() const { +return Impl.second; + } + + void print(llvm::raw_ostream &OS, PrinterHelper *Helper, PrintingPolicy &PP) { +OS << '(' << getLocationContext() << ',' << getAnyASTNodePtr() << ") "; +if
[PATCH] D47393: [clang-format] Disable AlwaysBreakBeforeMultilineStrings in Google style for Objective-C 📜
stephanemoore updated this revision to Diff 149338. stephanemoore added a comment. Removed the last format from the tests. That particular scenario was failing and it might need additional changes to pass. I also think that particular scenario is not critical to the change and can be considered in a followup. Repository: rC Clang https://reviews.llvm.org/D47393 Files: lib/Format/Format.cpp unittests/Format/FormatTestObjC.cpp Index: unittests/Format/FormatTestObjC.cpp === --- unittests/Format/FormatTestObjC.cpp +++ unittests/Format/FormatTestObjC.cpp @@ -1193,6 +1193,17 @@ "}"); } +TEST_F(FormatTestObjC, AlwaysBreakBeforeMultilineStrings) { + Style = getGoogleStyle(FormatStyle::LK_ObjC); + Style.ColumnLimit = 40; + verifyFormat(" = @\"\"\n" + " @\"\";"); + verifyFormat("(@\"\"\n" + " @\"\");"); + verifyFormat("(qqq, @\"\"\n" + " @\"\");"); +} + } // end namespace } // end namespace format } // end namespace clang Index: lib/Format/Format.cpp === --- lib/Format/Format.cpp +++ lib/Format/Format.cpp @@ -799,6 +799,7 @@ // has been implemented. GoogleStyle.BreakStringLiterals = false; } else if (Language == FormatStyle::LK_ObjC) { +GoogleStyle.AlwaysBreakBeforeMultilineStrings = false; GoogleStyle.ColumnLimit = 100; } Index: unittests/Format/FormatTestObjC.cpp === --- unittests/Format/FormatTestObjC.cpp +++ unittests/Format/FormatTestObjC.cpp @@ -1193,6 +1193,17 @@ "}"); } +TEST_F(FormatTestObjC, AlwaysBreakBeforeMultilineStrings) { + Style = getGoogleStyle(FormatStyle::LK_ObjC); + Style.ColumnLimit = 40; + verifyFormat(" = @\"\"\n" + " @\"\";"); + verifyFormat("(@\"\"\n" + " @\"\");"); + verifyFormat("(qqq, @\"\"\n" + " @\"\");"); +} + } // end namespace } // end namespace format } // end namespace clang Index: lib/Format/Format.cpp === --- lib/Format/Format.cpp +++ lib/Format/Format.cpp @@ -799,6 +799,7 @@ // has been implemented. GoogleStyle.BreakStringLiterals = false; } else if (Language == FormatStyle::LK_ObjC) { +GoogleStyle.AlwaysBreakBeforeMultilineStrings = false; GoogleStyle.ColumnLimit = 100; } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46013: [ARM] Conform to AAPCS when passing overaligned composites as arguments
efriedma added reviewers: aemerson, rjmccall. efriedma added a comment. I'm not sure Apple will want to mess with their ABI like this... adding some reviewers. Otherwise LGTM. Comment at: lib/CodeGen/TargetInfo.cpp:5790 // than ABI alignment. - uint64_t ABIAlign = 4; - uint64_t TyAlign = getContext().getTypeAlign(Ty) / 8; - if (getABIKind() == ARMABIInfo::AAPCS_VFP || - getABIKind() == ARMABIInfo::AAPCS) -ABIAlign = std::min(std::max(TyAlign, (uint64_t)4), (uint64_t)8); - + uint64_t ABIAlign = 32; + uint64_t TyAlign; I'd rather do alignment computations in bytes, rather than bits (can we use getTypeAlignInChars here?) https://reviews.llvm.org/D46013 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47157: Warning for framework headers using double quote includes
dexonsmith added inline comments. Comment at: test/Modules/double-quotes.m:27-29 +// CHECK: double-quoted include "A0.h" in framework header, expected angle-bracketed include instead +// CHECK: double-quoted include "B.h" in framework header, expected angle-bracketed include instead +// CHECK: double-quoted include "B.h" in framework header, expected angle-bracketed include instead aaron.ballman wrote: > dexonsmith wrote: > > When there's a fixit, you don't need to list it in the warning text (the > > fix-it itself is sufficient). I also feel like "quoted include" is as > > clear as "double-quoted include" (but more succinct). So I think these > > would be better as: > > > > > warning: quoted include "..." in framework header, expected > > > angle-bracketed include instead > > > Some other lexer diagnostics use "double-quoted" when they want to > distinguish with "angle-bracketed" (see > `warn_pragma_include_alias_mismatch_angle` and > `warn_pragma_include_alias_mismatch_quote` as examples). I don't have a > strong opinion on what form we use, but I'd prefer for it to be consistent > exposition. I agree we should be consistent. No reason to change it here then. https://reviews.llvm.org/D47157 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47157: Warning for framework headers using double quote includes
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D47157 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47603: [analyzer] fix bug with 1-bit APSInt types in Z3ConstraintManager
ddcc created this revision. ddcc added reviewers: george.karpenkov, NoQ. Herald added subscribers: a.sidorin, szepet, xazax.hun. Clang does not have a corresponding QualType for a 1-bit APSInt, so use the BoolTy and extend the APSInt. Split from https://reviews.llvm.org/D35450. Repository: rC Clang https://reviews.llvm.org/D47603 Files: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp Index: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp === --- lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp +++ lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp @@ -987,6 +987,10 @@ // TODO: Refactor to put elsewhere QualType getAPSIntType(const llvm::APSInt &Int) const; + // Fix the input APSInt if it is has a bitwidth of 1, and set the type. + const llvm::APSInt &fixAPSInt(const llvm::APSInt &Int, llvm::APSInt &NewInt, +QualType *Ty = nullptr) const; + // Perform implicit type conversion on binary symbolic expressions. // May modify all input parameters. // TODO: Refactor to use built-in conversion functions @@ -1037,28 +1041,29 @@ QualType RetTy; // The expression may be casted, so we cannot call getZ3DataExpr() directly Z3Expr Exp = getZ3Expr(Sym, &RetTy); - - assert((getAPSIntType(From) == getAPSIntType(To)) && - "Range values have different types!"); - QualType RTy = getAPSIntType(From); - bool isSignedTy = RetTy->isSignedIntegerOrEnumerationType(); - Z3Expr FromExp = Z3Expr::fromAPSInt(From); - Z3Expr ToExp = Z3Expr::fromAPSInt(To); + QualType LTy; + llvm::APSInt NewFromInt; + Z3Expr FromExp = Z3Expr::fromAPSInt(fixAPSInt(From, NewFromInt,isSignedIntegerOrEnumerationType())); } ProgramStateRef Z3ConstraintManager::assumeSymUnsupported(ProgramStateRef State, @@ -1145,8 +1150,8 @@ const llvm::APSInt *Z3ConstraintManager::getSymVal(ProgramStateRef State, SymbolRef Sym) const { - BasicValueFactory &BV = getBasicVals(); - ASTContext &Ctx = BV.getContext(); + BasicValueFactory &BVF = getBasicVals(); + ASTContext &Ctx = BVF.getContext(); if (const SymbolData *SD = dyn_cast(Sym)) { QualType Ty = Sym->getType(); @@ -1180,7 +1185,7 @@ return nullptr; // This is the only solution, store it -return &BV.getValue(Value); +return &BVF.getValue(Value); } else if (const SymbolCast *SC = dyn_cast(Sym)) { SymbolRef CastSym = SC->getOperand(); QualType CastTy = SC->getType(); @@ -1191,7 +1196,7 @@ const llvm::APSInt *Value; if (!(Value = getSymVal(State, CastSym))) return nullptr; -return &BV.Convert(SC->getType(), *Value); +return &BVF.Convert(SC->getType(), *Value); } else if (const BinarySymExpr *BSE = dyn_cast(Sym)) { const llvm::APSInt *LHS, *RHS; if (const SymIntExpr *SIE = dyn_cast(BSE)) { @@ -1215,7 +1220,7 @@ QualType LTy = getAPSIntType(*LHS), RTy = getAPSIntType(*RHS); doIntTypeConversion( ConvertedLHS, LTy, ConvertedRHS, RTy); -return BV.evalAPSInt(BSE->getOpcode(), ConvertedLHS, ConvertedRHS); +return BVF.evalAPSInt(BSE->getOpcode(), ConvertedLHS, ConvertedRHS); } llvm_unreachable("Unsupported expression to get symbol value!"); @@ -1342,13 +1347,13 @@ BinaryOperator::Opcode Op = BSE->getOpcode(); if (const SymIntExpr *SIE = dyn_cast(BSE)) { -RTy = getAPSIntType(SIE->getRHS()); +llvm::APSInt NewRInt; Z3Expr LHS = getZ3SymExpr(SIE->getLHS(), getRHS()); +Z3Expr RHS = Z3Expr::fromAPSInt(fixAPSInt(SIE->getRHS(), NewRInt, &RTy)); return getZ3BinExpr(LHS, LTy, Op, RHS, RTy, RetTy); } else if (const IntSymExpr *ISE = dyn_cast(BSE)) { -LTy = getAPSIntType(ISE->getLHS()); -Z3Expr LHS = Z3Expr::fromAPSInt(ISE->getLHS()); +llvm::APSInt NewLInt; +Z3Expr LHS = Z3Expr::fromAPSInt(fixAPSInt(ISE->getLHS
Re: [PATCH] D47597: IRGen: Write .dwo files when -split-dwarf-file is used together with -fthinlto-index.
On Thu, May 31, 2018 at 12:00 PM Peter Collingbourne wrote: > > > On Thu, May 31, 2018 at 11:54 AM, David Blaikie > wrote: > >> >> >> On Thu, May 31, 2018 at 11:20 AM Peter Collingbourne via Phabricator < >> revi...@reviews.llvm.org> wrote: >> >>> pcc created this revision. >>> pcc added reviewers: tejohnson, dblaikie. >>> Herald added subscribers: JDevlieghere, hiraditya, eraman, inglorion, >>> mehdi_amini. >>> >>> https://reviews.llvm.org/D47597 >>> >>> Files: >>> clang/lib/CodeGen/BackendUtil.cpp >>> clang/test/CodeGen/thinlto-split-dwarf.c >>> llvm/include/llvm/LTO/Config.h >>> llvm/lib/LTO/LTOBackend.cpp >>> >>> >>> Index: llvm/lib/LTO/LTOBackend.cpp >>> === >>> --- llvm/lib/LTO/LTOBackend.cpp >>> +++ llvm/lib/LTO/LTOBackend.cpp >>> @@ -291,14 +291,19 @@ >>> return; >>> >>>std::unique_ptr DwoOut; >>> + SmallString<1024> DwoFile(Conf.DwoPath); >>>if (!Conf.DwoDir.empty()) { >>> std::error_code EC; >>> if (auto EC = llvm::sys::fs::create_directories(Conf.DwoDir)) >>>report_fatal_error("Failed to create directory " + Conf.DwoDir + >>> ": " + >>> EC.message()); >>> >>> -SmallString<1024> DwoFile(Conf.DwoDir); >>> +DwoFile = Conf.DwoDir; >>> sys::path::append(DwoFile, std::to_string(Task) + ".dwo"); >>> + } >>> + >>> + if (!DwoFile.empty()) { >>> +std::error_code EC; >>> TM->Options.MCOptions.SplitDwarfFile = DwoFile.str().str(); >>> DwoOut = llvm::make_unique(DwoFile, EC, >>> sys::fs::F_None); >>> if (EC) >>> Index: llvm/include/llvm/LTO/Config.h >>> === >>> --- llvm/include/llvm/LTO/Config.h >>> +++ llvm/include/llvm/LTO/Config.h >>> @@ -76,6 +76,11 @@ >>>/// The directory to store .dwo files. >>>std::string DwoDir; >>> >>> + /// The path to write a .dwo file to. This should generally only be >>> used when >>> + /// running an individual backend directly via thinBackend(), as >>> otherwise >>> + /// all .dwo files will be written to the same path. >>> + std::string DwoPath; >>> + >>>/// Optimization remarks file path. >>>std::string RemarksFilename = ""; >>> >>> Index: clang/test/CodeGen/thinlto-split-dwarf.c >>> === >>> --- /dev/null >>> +++ clang/test/CodeGen/thinlto-split-dwarf.c >>> @@ -0,0 +1,21 @@ >>> +// REQUIRES: x86-registered-target >>> + >>> +// RUN: %clang_cc1 -debug-info-kind=limited -triple >>> x86_64-unknown-linux-gnu \ >>> +// RUN: -flto=thin -emit-llvm-bc \ >>> +// RUN: -o %t.o %s >>> + >>> +// RUN: llvm-lto2 run -thinlto-distributed-indexes %t.o \ >>> +// RUN: -o %t2.index \ >>> +// RUN: -r=%t.o,main,px >>> + >>> +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu \ >>> +// RUN: -emit-obj -fthinlto-index=%t.o.thinlto.bc \ >>> +// RUN: -o %t.native.o -split-dwarf-file %t.native.dwo -x ir %t.o >>> >> >> Can this be written in a single IR file yet (rather than frontend >> compiling, indexing, then backend compiling), now that Teresa's implemented >> some of the summary IR syntax? >> > > I think the parsing part isn't implemented yet, so that wouldn't work. > Regarding this - the parsing support is coming along, slowly but surely. I'd say I am about 75% done. Teresa > Besides, we would need two separate files anyway: the bitcode file (%t.o) > and the combined summary (%t.o.thinlto.bc). > > Peter > > >> >> >>> + >>> +// RUN: llvm-readobj -sections %t.native.o | FileCheck --check-prefix=O >>> %s >>> +// RUN: llvm-readobj -sections %t.native.dwo | FileCheck >>> --check-prefix=DWO %s >>> + >>> +// O-NOT: .dwo >>> +// DWO: .dwo >>> + >>> +int main() {} >>> Index: clang/lib/CodeGen/BackendUtil.cpp >>> === >>> --- clang/lib/CodeGen/BackendUtil.cpp >>> +++ clang/lib/CodeGen/BackendUtil.cpp >>> @@ -1161,6 +1161,7 @@ >>>Conf.DebugPassManager = CGOpts.DebugPassManager; >>>Conf.RemarksWithHotness = CGOpts.DiagnosticsWithHotness; >>>Conf.RemarksFilename = CGOpts.OptRecordFile; >>> + Conf.DwoPath = CGOpts.SplitDwarfFile; >>>switch (Action) { >>>case Backend_EmitNothing: >>> Conf.PreCodeGenModuleHook = [](size_t Task, const Module &Mod) { >>> >>> >>> > > > -- > -- > Peter > -- Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47603: [analyzer] fix bug with 1-bit APSInt types in Z3ConstraintManager
george.karpenkov requested changes to this revision. george.karpenkov added a comment. This revision now requires changes to proceed. Thanks! Looks good with minor changes. Would it be possible to add tests? I know we have very few unit tests, but I assume you could actually use an integration test to exercise this path? Comment at: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp:1044 Z3Expr Exp = getZ3Expr(Sym, &RetTy); - - assert((getAPSIntType(From) == getAPSIntType(To)) && - "Range values have different types!"); - QualType RTy = getAPSIntType(From); - bool isSignedTy = RetTy->isSignedIntegerOrEnumerationType(); - Z3Expr FromExp = Z3Expr::fromAPSInt(From); - Z3Expr ToExp = Z3Expr::fromAPSInt(To); + QualType LTy; + llvm::APSInt NewFromInt; What does `L` stand for here? It's confusing because `L/R` usually stand for left/right-hand-side in this context. Comment at: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp:1154 + BasicValueFactory &BVF = getBasicVals(); + ASTContext &Ctx = BVF.getContext(); that's a separate change, but OK Comment at: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp:1426 +*Ty = getAPSIntType(Int); + return Int; +} It's redundant to mutate the argument passed by reference and also return it. Could we take a single `APSInt` parameter by value and return `std::pair` ? Comment at: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp:1640 #else - llvm::report_fatal_error("Clang was not compiled with Z3 support!", false); + llvm::report_fatal_error("Clang was not compiled with Z3 support, rebuild " + "with -DCLANG_ANALYZER_BUILD_Z3=ON", that's a separate change, but OK Repository: rC Clang https://reviews.llvm.org/D47603 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47350: [analyzer] Track class member initializer constructors path-sensitively within their construction context.
george.karpenkov added a comment. 👍 https://reviews.llvm.org/D47350 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47564: [Parse] Use CapturedStmt for @finally on MSVC
rnk added a comment. Using CapturedStmt to do frontend outlining was the direction I suggested. I want to hear what @rsmith and @rjmccall think, though. Comment at: lib/Parse/ParseObjc.cpp:2588 + bool ShouldCapture = Actions.getASTContext() + .getTargetInfo() Parser has a `getTargetInfo()` method, so this can be shorter. Repository: rC Clang https://reviews.llvm.org/D47564 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47603: [analyzer] fix bug with 1-bit APSInt types in Z3ConstraintManager
vlad.tsyrklevich added a comment. In https://reviews.llvm.org/D47603#1118106, @george.karpenkov wrote: > Would it be possible to add tests? I know we have very few unit tests, but I > assume you could actually use an integration test to exercise this path? I tested this change and it fixes PR37622. There's a simple crash reproducer included there. Repository: rC Clang https://reviews.llvm.org/D47603 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47564: [Parse] Use CapturedStmt for @finally on MSVC
smeenai updated this revision to Diff 149351. smeenai edited the summary of this revision. smeenai added a comment. @rnk comment Repository: rC Clang https://reviews.llvm.org/D47564 Files: include/clang/AST/Stmt.h include/clang/Basic/CapturedStmt.h include/clang/Sema/ScopeInfo.h lib/Parse/ParseObjc.cpp test/SemaObjC/finally-msvc.m Index: test/SemaObjC/finally-msvc.m === --- /dev/null +++ test/SemaObjC/finally-msvc.m @@ -0,0 +1,14 @@ +// RUN: %clang_cc1 -triple i686--windows-msvc -fexceptions -fobjc-exceptions -ast-dump %s 2>&1 | FileCheck %s +// RUN: %clang_cc1 -triple x86_64--windows-msvc -fexceptions -fobjc-exceptions -ast-dump %s 2>&1 | FileCheck %s + +void f() { + @try { + } @finally { + } +} + +// CHECK: ObjCAtFinallyStmt +// CHECK-NEXT: CapturedStmt +// CHECK-NEXT: CapturedDecl +// CHECK-NEXT: CompoundStmt +// CHECK-NEXT: ImplicitParamDecl Index: lib/Parse/ParseObjc.cpp === --- lib/Parse/ParseObjc.cpp +++ lib/Parse/ParseObjc.cpp @@ -2585,13 +2585,26 @@ ParseScope FinallyScope(this, Scope::DeclScope | Scope::CompoundStmtScope); + bool ShouldCapture = + getTargetInfo().getTriple().isWindowsMSVCEnvironment(); + if (ShouldCapture) +Actions.ActOnCapturedRegionStart(Tok.getLocation(), getCurScope(), + CR_ObjCAtFinally, 1); + StmtResult FinallyBody(true); if (Tok.is(tok::l_brace)) FinallyBody = ParseCompoundStatementBody(); else Diag(Tok, diag::err_expected) << tok::l_brace; - if (FinallyBody.isInvalid()) + + if (FinallyBody.isInvalid()) { FinallyBody = Actions.ActOnNullStmt(Tok.getLocation()); +if (ShouldCapture) + Actions.ActOnCapturedRegionError(); + } else if (ShouldCapture) { +FinallyBody = Actions.ActOnCapturedRegionEnd(FinallyBody.get()); + } + FinallyStmt = Actions.ActOnObjCAtFinallyStmt(AtCatchFinallyLoc, FinallyBody.get()); catch_or_finally_seen = true; Index: include/clang/Sema/ScopeInfo.h === --- include/clang/Sema/ScopeInfo.h +++ include/clang/Sema/ScopeInfo.h @@ -748,6 +748,8 @@ switch (CapRegionKind) { case CR_Default: return "default captured statement"; +case CR_ObjCAtFinally: + return "Objective-C @finally statement"; case CR_OpenMP: return "OpenMP region"; } Index: include/clang/Basic/CapturedStmt.h === --- include/clang/Basic/CapturedStmt.h +++ include/clang/Basic/CapturedStmt.h @@ -16,6 +16,7 @@ /// The different kinds of captured statement. enum CapturedRegionKind { CR_Default, + CR_ObjCAtFinally, CR_OpenMP }; Index: include/clang/AST/Stmt.h === --- include/clang/AST/Stmt.h +++ include/clang/AST/Stmt.h @@ -2133,7 +2133,7 @@ /// The pointer part is the implicit the outlined function and the /// int part is the captured region kind, 'CR_Default' etc. - llvm::PointerIntPair CapDeclAndKind; + llvm::PointerIntPair CapDeclAndKind; /// The record for captured variables, a RecordDecl or CXXRecordDecl. RecordDecl *TheRecordDecl = nullptr; Index: test/SemaObjC/finally-msvc.m === --- /dev/null +++ test/SemaObjC/finally-msvc.m @@ -0,0 +1,14 @@ +// RUN: %clang_cc1 -triple i686--windows-msvc -fexceptions -fobjc-exceptions -ast-dump %s 2>&1 | FileCheck %s +// RUN: %clang_cc1 -triple x86_64--windows-msvc -fexceptions -fobjc-exceptions -ast-dump %s 2>&1 | FileCheck %s + +void f() { + @try { + } @finally { + } +} + +// CHECK: ObjCAtFinallyStmt +// CHECK-NEXT: CapturedStmt +// CHECK-NEXT: CapturedDecl +// CHECK-NEXT: CompoundStmt +// CHECK-NEXT: ImplicitParamDecl Index: lib/Parse/ParseObjc.cpp === --- lib/Parse/ParseObjc.cpp +++ lib/Parse/ParseObjc.cpp @@ -2585,13 +2585,26 @@ ParseScope FinallyScope(this, Scope::DeclScope | Scope::CompoundStmtScope); + bool ShouldCapture = + getTargetInfo().getTriple().isWindowsMSVCEnvironment(); + if (ShouldCapture) +Actions.ActOnCapturedRegionStart(Tok.getLocation(), getCurScope(), + CR_ObjCAtFinally, 1); + StmtResult FinallyBody(true); if (Tok.is(tok::l_brace)) FinallyBody = ParseCompoundStatementBody(); else Diag(Tok, diag::err_expected) << tok::l_brace; - if (FinallyBody.isInvalid()) + + if (FinallyBody.isInvalid()) { FinallyBody = Actions.ActOnNullStmt(Tok.
[PATCH] D47603: [analyzer] fix bug with 1-bit APSInt types in Z3ConstraintManager
ddcc added a comment. In https://reviews.llvm.org/D47603#1118138, @vlad.tsyrklevich wrote: > In https://reviews.llvm.org/D47603#1118106, @george.karpenkov wrote: > > > Would it be possible to add tests? I know we have very few unit tests, but > > I assume you could actually use an integration test to exercise this path? > > > I tested this change and it fixes PR37622. There's a simple crash reproducer > included there. Cool, thanks for the repro! It's been long enough since I've touched this code that I don't recall the original failing testcase. I'll add the test to this revision. Comment at: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp:1044 Z3Expr Exp = getZ3Expr(Sym, &RetTy); - - assert((getAPSIntType(From) == getAPSIntType(To)) && - "Range values have different types!"); - QualType RTy = getAPSIntType(From); - bool isSignedTy = RetTy->isSignedIntegerOrEnumerationType(); - Z3Expr FromExp = Z3Expr::fromAPSInt(From); - Z3Expr ToExp = Z3Expr::fromAPSInt(To); + QualType LTy; + llvm::APSInt NewFromInt; george.karpenkov wrote: > What does `L` stand for here? It's confusing because `L/R` usually stand for > left/right-hand-side in this context. They correspond to the inferred left/right hand-side inferred types, but inside the subsequent Z3Expr `LHS` and `RHS` variables. This is confusing, so I'll rename them to `FromTy` and `ToTy`. Comment at: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp:1154 + BasicValueFactory &BVF = getBasicVals(); + ASTContext &Ctx = BVF.getContext(); george.karpenkov wrote: > that's a separate change, but OK Yeah, this is one of several small miscellaneous changes that didn't make the original commit. It seemed a bit excessive to open separate revisions for each, so I've just been merging them into the next patch. I'm not sure which is preferable? Comment at: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp:1426 +*Ty = getAPSIntType(Int); + return Int; +} george.karpenkov wrote: > It's redundant to mutate the argument passed by reference and also return it. > Could we take a single `APSInt` parameter by value and return > `std::pair` ? Sure. Repository: rC Clang https://reviews.llvm.org/D47603 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47603: [analyzer] fix bug with 1-bit APSInt types in Z3ConstraintManager
ddcc updated this revision to Diff 149356. ddcc added a comment. Add test, address comments Repository: rC Clang https://reviews.llvm.org/D47603 Files: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp test/Analysis/apsint.c Index: test/Analysis/apsint.c === --- /dev/null +++ test/Analysis/apsint.c @@ -0,0 +1,7 @@ +// REQUIRES: z3 +// RUN: %clang_analyze_cc1 -triple x86_64-unknown-linux-gnu -analyzer-checker=core -verify %s +// expected-no-diagnostics + +_Bool a() { + return !({ a(); }); +} Index: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp === --- lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp +++ lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp @@ -987,6 +987,9 @@ // TODO: Refactor to put elsewhere QualType getAPSIntType(const llvm::APSInt &Int) const; + // Get the QualTy for the input APSInt, and fix it if it has a bitwidth of 1. + std::pair fixAPSInt(const llvm::APSInt &Int) const; + // Perform implicit type conversion on binary symbolic expressions. // May modify all input parameters. // TODO: Refactor to use built-in conversion functions @@ -1038,27 +1041,31 @@ // The expression may be casted, so we cannot call getZ3DataExpr() directly Z3Expr Exp = getZ3Expr(Sym, &RetTy); - assert((getAPSIntType(From) == getAPSIntType(To)) && - "Range values have different types!"); - QualType RTy = getAPSIntType(From); - bool isSignedTy = RetTy->isSignedIntegerOrEnumerationType(); - Z3Expr FromExp = Z3Expr::fromAPSInt(From); - Z3Expr ToExp = Z3Expr::fromAPSInt(To); + QualType FromTy; + llvm::APSInt NewFromInt; + std::tie(NewFromInt, FromTy) = fixAPSInt(From); + Z3Expr FromExp = Z3Expr::fromAPSInt(NewFromInt); // Construct single (in)equality if (From == To) return assumeZ3Expr(State, Sym, getZ3BinExpr(Exp, RetTy, InRange ? BO_EQ : BO_NE, - FromExp, RTy, nullptr)); + FromExp, FromTy, nullptr)); + QualType ToTy; + llvm::APSInt NewToInt; + std::tie(NewToInt, ToTy) = fixAPSInt(To); + Z3Expr ToExp = Z3Expr::fromAPSInt(NewToInt); + assert(FromTy == ToTy && "Range values have different types!"); // Construct two (in)equalities, and a logical and/or - Z3Expr LHS = - getZ3BinExpr(Exp, RetTy, InRange ? BO_GE : BO_LT, FromExp, RTy, nullptr); + Z3Expr LHS = getZ3BinExpr(Exp, RetTy, InRange ? BO_GE : BO_LT, FromExp, +FromTy, nullptr); Z3Expr RHS = - getZ3BinExpr(Exp, RetTy, InRange ? BO_LE : BO_GT, ToExp, RTy, nullptr); + getZ3BinExpr(Exp, RetTy, InRange ? BO_LE : BO_GT, ToExp, ToTy, nullptr); return assumeZ3Expr( State, Sym, - Z3Expr::fromBinOp(LHS, InRange ? BO_LAnd : BO_LOr, RHS, isSignedTy)); + Z3Expr::fromBinOp(LHS, InRange ? BO_LAnd : BO_LOr, RHS, +RetTy->isSignedIntegerOrEnumerationType())); } ProgramStateRef Z3ConstraintManager::assumeSymUnsupported(ProgramStateRef State, @@ -1145,8 +1152,8 @@ const llvm::APSInt *Z3ConstraintManager::getSymVal(ProgramStateRef State, SymbolRef Sym) const { - BasicValueFactory &BV = getBasicVals(); - ASTContext &Ctx = BV.getContext(); + BasicValueFactory &BVF = getBasicVals(); + ASTContext &Ctx = BVF.getContext(); if (const SymbolData *SD = dyn_cast(Sym)) { QualType Ty = Sym->getType(); @@ -1180,7 +1187,7 @@ return nullptr; // This is the only solution, store it -return &BV.getValue(Value); +return &BVF.getValue(Value); } else if (const SymbolCast *SC = dyn_cast(Sym)) { SymbolRef CastSym = SC->getOperand(); QualType CastTy = SC->getType(); @@ -1191,7 +1198,7 @@ const llvm::APSInt *Value; if (!(Value = getSymVal(State, CastSym))) return nullptr; -return &BV.Convert(SC->getType(), *Value); +return &BVF.Convert(SC->getType(), *Value); } else if (const BinarySymExpr *BSE = dyn_cast(Sym)) { const llvm::APSInt *LHS, *RHS; if (const SymIntExpr *SIE = dyn_cast(BSE)) { @@ -1215,7 +1222,7 @@ QualType LTy = getAPSIntType(*LHS), RTy = getAPSIntType(*RHS); doIntTypeConversion( ConvertedLHS, LTy, ConvertedRHS, RTy); -return BV.evalAPSInt(BSE->getOpcode(), ConvertedLHS, ConvertedRHS); +return BVF.evalAPSInt(BSE->getOpcode(), ConvertedLHS, ConvertedRHS); } llvm_unreachable("Unsupported expression to get symbol value!"); @@ -1342,13 +1349,15 @@ BinaryOperator::Opcode Op = BSE->getOpcode(); if (const SymIntExpr *SIE = dyn_cast(BSE)) { -RTy = getAPSIntType(SIE->getRHS()); Z3Expr LHS = getZ3SymExpr(SIE->getLHS(),getRHS()); +llvm::APSInt NewRInt; +std::tie(NewRInt, RTy) = fixAPSInt(SIE->getRHS()); +Z3Expr RHS = Z3Expr::fromAPSInt(NewRInt); return
[PATCH] D47157: Warning for framework headers using double quote includes
bruno updated this revision to Diff 149359. bruno edited the summary of this revision. bruno added a comment. Update after Duncan's review: remove header name from the warning message (since it's already in the fixit) https://reviews.llvm.org/D47157 Files: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticLexKinds.td lib/Lex/HeaderSearch.cpp test/Modules/Inputs/double-quotes/A.framework/Headers/A.h test/Modules/Inputs/double-quotes/A.framework/Headers/A0.h test/Modules/Inputs/double-quotes/A.framework/Modules/module.modulemap test/Modules/Inputs/double-quotes/B.h test/Modules/Inputs/double-quotes/X.framework/Headers/X.h test/Modules/Inputs/double-quotes/X.framework/Modules/module.modulemap test/Modules/Inputs/double-quotes/a.hmap.json test/Modules/Inputs/double-quotes/flat-header-path/Z.h test/Modules/Inputs/double-quotes/flat-header-path/Z.modulemap test/Modules/Inputs/double-quotes/x.hmap.json test/Modules/Inputs/double-quotes/z.yaml test/Modules/double-quotes.m Index: test/Modules/double-quotes.m === --- /dev/null +++ test/Modules/double-quotes.m @@ -0,0 +1,39 @@ +// REQUIRES: shell + +// RUN: rm -rf %t +// RUN: mkdir %t + +// RUN: hmaptool write %S/Inputs/double-quotes/a.hmap.json %t/a.hmap +// RUN: hmaptool write %S/Inputs/double-quotes/x.hmap.json %t/x.hmap + +// RUN: sed -e "s:TEST_DIR:%S/Inputs/double-quotes:g" \ +// RUN: %S/Inputs/double-quotes/z.yaml > %t/z.yaml + +// The output with and without modules should be the same + +// RUN: %clang_cc1 \ +// RUN: -I %t/x.hmap -iquote %t/a.hmap -ivfsoverlay %t/z.yaml \ +// RUN: -F%S/Inputs/double-quotes -I%S/Inputs/double-quotes \ +// RUN: -Wquoted-include-in-framework-header -fsyntax-only %s -verify + +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache \ +// RUN: -I %t/x.hmap -iquote %t/a.hmap -ivfsoverlay %t/z.yaml \ +// RUN: -F%S/Inputs/double-quotes -I%S/Inputs/double-quotes \ +// RUN: -Wquoted-include-in-framework-header -fsyntax-only %s \ +// RUN: 2>%t/stderr + +// The same warnings show up when modules is on but -verify doesn't get it +// because they only show up under the module A building context. +// RUN: FileCheck --input-file=%t/stderr %s +// CHECK: double-quoted include "A0.h" in framework header, expected angle-bracketed instead +// CHECK: double-quoted include "B.h" in framework header, expected angle-bracketed instead +// CHECK: double-quoted include "B.h" in framework header, expected angle-bracketed instead + +#import "A.h" +#import + +int bar() { return foo(); } + +// expected-warning@Inputs/double-quotes/A.framework/Headers/A.h:1{{double-quoted include "A0.h" in framework header, expected angle-bracketed instead}} +// expected-warning@Inputs/double-quotes/A.framework/Headers/A.h:2{{double-quoted include "B.h" in framework header, expected angle-bracketed instead}} +// expected-warning@Inputs/double-quotes/flat-header-path/Z.h:1{{double-quoted include "B.h" in framework header, expected angle-bracketed instead}} Index: test/Modules/Inputs/double-quotes/z.yaml === --- /dev/null +++ test/Modules/Inputs/double-quotes/z.yaml @@ -0,0 +1,28 @@ +{ + 'version': 0, + 'case-sensitive': 'false', + 'roots': [ +{ + 'type': 'directory', + 'name': "TEST_DIR/Z.framework/Headers", + 'contents': [ +{ + 'type': 'file', + 'name': "Z.h", + 'external-contents': "TEST_DIR/flat-header-path/Z.h" +} + ] +}, +{ + 'type': 'directory', + 'name': "TEST_DIR/Z.framework/Modules", + 'contents': [ +{ + 'type': 'file', + 'name': "module.modulemap", + 'external-contents': "TEST_DIR/flat-header-path/Z.modulemap" +} + ] +} + ] +} Index: test/Modules/Inputs/double-quotes/x.hmap.json === --- /dev/null +++ test/Modules/Inputs/double-quotes/x.hmap.json @@ -0,0 +1,7 @@ + +{ + "mappings" : +{ + "X.h" : "X/X.h" +} +} Index: test/Modules/Inputs/double-quotes/flat-header-path/Z.modulemap === --- /dev/null +++ test/Modules/Inputs/double-quotes/flat-header-path/Z.modulemap @@ -0,0 +1,3 @@ +framework module Z { + header "Z.h" +} Index: test/Modules/Inputs/double-quotes/flat-header-path/Z.h === --- /dev/null +++ test/Modules/Inputs/double-quotes/flat-header-path/Z.h @@ -0,0 +1 @@ +#import "B.h" // Included from Z.h & A.h Index: test/Modules/Inputs/double-quotes/a.hmap.json === --- /dev/null +++ test/Modules/Inputs/double-quotes/a.hmap.json @@ -0,0 +1,7 @@ + +{ + "mappings" : +{ + "A.h" : "A/A.h" +} +} Index: test/Modules/Inputs/double-quot
[PATCH] D47157: Warning for framework headers using double quote includes
bruno added inline comments. Comment at: test/Modules/double-quotes.m:24-25 + +// The same warnings show up when modules is on but -verify doesn't get it +// because they only show up under the module A building context. +// RUN: FileCheck --input-file=%t/stderr %s dexonsmith wrote: > Would using an explicit module build make this any easier? --- Not really =T https://reviews.llvm.org/D47157 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46911: [Fixed Point Arithmetic] Addition of the remaining fixed point types and their saturated equivalents
leonardchan updated this revision to Diff 149360. leonardchan marked 7 inline comments as done. Repository: rC Clang https://reviews.llvm.org/D46911 Files: include/clang/AST/ASTContext.h include/clang/AST/BuiltinTypes.def include/clang/AST/Type.h include/clang/Basic/DiagnosticSemaKinds.td include/clang/Basic/Specifiers.h include/clang/Basic/TargetInfo.h include/clang/Basic/TokenKinds.def include/clang/Sema/DeclSpec.h include/clang/Serialization/ASTBitCodes.h lib/AST/ASTContext.cpp lib/AST/ExprConstant.cpp lib/AST/ItaniumMangle.cpp lib/AST/MicrosoftMangle.cpp lib/AST/NSAPI.cpp lib/AST/Type.cpp lib/AST/TypeLoc.cpp lib/Analysis/PrintfFormatString.cpp lib/Basic/TargetInfo.cpp lib/CodeGen/CGDebugInfo.cpp lib/CodeGen/CodeGenTypes.cpp lib/CodeGen/ItaniumCXXABI.cpp lib/Index/USRGeneration.cpp lib/Parse/ParseDecl.cpp lib/Sema/DeclSpec.cpp lib/Sema/SemaTemplateVariadic.cpp lib/Sema/SemaType.cpp lib/Serialization/ASTCommon.cpp lib/Serialization/ASTReader.cpp test/Frontend/fixed_point.c test/Frontend/fixed_point_bit_widths.c test/Frontend/fixed_point_errors.c test/Frontend/fixed_point_errors.cpp Index: test/Frontend/fixed_point_errors.cpp === --- test/Frontend/fixed_point_errors.cpp +++ test/Frontend/fixed_point_errors.cpp @@ -1,5 +1,9 @@ +// RUN: %clang_cc1 -x c++ %s -verify // RUN: %clang_cc1 -x c++ -ffixed-point %s -verify // Name namgling is not provided for fixed point types in c++ _Accum accum; // expected-error{{unknown type name '_Accum'}} +_Fract fract; // expected-error{{unknown type name '_Fract'}} +_Sat _Accum sat_accum; // expected-error{{unknown type name '_Sat'}} +// expected-error@-1{{expected ';' after top level declarator}} Index: test/Frontend/fixed_point_errors.c === --- test/Frontend/fixed_point_errors.c +++ test/Frontend/fixed_point_errors.c @@ -5,6 +5,14 @@ long long _Accum longlong_accum; // expected-error{{'long long _Accum' is invalid}} unsigned long long _Accum u_longlong_accum; // expected-error{{'long long _Accum' is invalid}} +long long _Fract longlong_fract; // expected-error{{'long long _Fract' is invalid}} +unsigned long long _Fract u_longlong_fract; // expected-error{{'long long _Fract' is invalid}} + +_Sat long long _Accum sat_longlong_accum; // expected-error{{'long long _Accum' is invalid}} +_Sat unsigned long long _Accum sat_u_longlong_accum; // expected-error{{'long long _Accum' is invalid}} +_Sat long long _Fract sat_longlong_fract; // expected-error{{'long long _Fract' is invalid}} +_Sat unsigned long long _Fract sat_u_longlong_fract; // expected-error{{'long long _Fract' is invalid}} + /* Although _Complex types work with floating point numbers, the extension * provides no info for complex fixed point types. */ @@ -19,9 +27,67 @@ _Complex _Accum cmplx_s_accum; // expected-error{{'_Complex _Accum' is invalid}} _Complex long _Accum cmplx_s_long_accum;// expected-error{{'_Complex _Accum' is invalid}} +_Complex signed short _Fract cmplx_s_short_fract; // expected-error{{'_Complex _Fract' is invalid}} +_Complex signed _Fract cmplx_s_fract; // expected-error{{'_Complex _Fract' is invalid}} +_Complex signed long _Fract cmplx_s_long_fract; // expected-error{{'_Complex _Fract' is invalid}} +_Complex unsigned short _Fract cmplx_u_short_fract; // expected-error{{'_Complex _Fract' is invalid}} +_Complex unsigned _Fract cmplx_u_fract; // expected-error{{'_Complex _Fract' is invalid}} +_Complex unsigned long _Fract cmplx_u_long_fract; // expected-error{{'_Complex _Fract' is invalid}} +_Complex short _Fract cmplx_s_short_fract; // expected-error{{'_Complex _Fract' is invalid}} +_Complex _Fract cmplx_s_fract; // expected-error{{'_Complex _Fract' is invalid}} +_Complex long _Fract cmplx_s_long_fract;// expected-error{{'_Complex _Fract' is invalid}} + +_Complex _Sat signed short _Accum cmplx_sat_s_short_accum; // expected-error{{'_Complex _Accum' is invalid}} +_Complex _Sat signed _Accum cmplx_sat_s_accum; // expected-error{{'_Complex _Accum' is invalid}} +_Complex _Sat signed long _Accum cmplx_sat_s_long_accum; // expected-error{{'_Complex _Accum' is invalid}} +_Complex _Sat unsigned short _Accum cmplx_sat_u_short_accum; // expected-error{{'_Complex _Accum' is invalid}} +_Complex _Sat unsigned _Accum cmplx_sat_u_accum; // expected-error{{'_Complex _Accum' is invalid}} +_Complex _Sat unsigned long _Accum cmplx_sat_u_long_accum; // expected-error{{'_Complex _Accum' is invalid}} +_Complex _Sat short _Accum cmplx_sat_s_short_accum; // expected-error{{'_Complex _Accum' is i
[PATCH] D46911: [Fixed Point Arithmetic] Addition of the remaining fixed point types and their saturated equivalents
leonardchan added inline comments. Comment at: lib/Sema/DeclSpec.cpp:1123 +if (!(TypeSpecType == TST_accum || TypeSpecType == TST_fract)) { + S.Diag(TSSatLoc, diag::err_invalid_saturation_spec) + << getSpecifierName((TST)TypeSpecType, Policy); ebevhan wrote: > Handling this case here means that placing _Sat on something other than > exactly a fixed-point type is a parsing error rather than a semantic error. > How does this handle _Sat on sugared types? Should _Sat on things like > typedefs work? > > typedef _Fract myfract; > _Sat myfract F; > > The primary issue (and this is one that we have encountered as well) is that > you cannot have a true _Sat typedef since _Sat only exists as part of builtin > types. You need to desugar/canonicalize the type and then do > getCorrespondingSaturatingType (or have getCorrespondingSaturatingType look > at the canonical type internally). I think _Sat is analogous to _Complex where it only works with specific builtin types, albeit the only builtin type _Complex doesn't work with is _Bool. Currently this example would throw the error `'_Sat' specifier is only valid on '_Fract' or '_Accum', not 'type-name'` which is similar to what _Complex does when paired with a typedef: ``` typedef double mydouble; mydouble _Complex D; // _Complex type-name' is invalid ``` I don't see this as a big problem right now, but am willing to come back to this in the future if it becomes more urgent. For now, I added a test that asserts this error is thrown. Comment at: lib/Sema/SemaType.cpp:1410 + +if (DS.getTypeSpecSign() != DeclSpec::TSS_unsigned) + Result = fixedpoint::getCorrespondingSignedType(Context, Result); ebevhan wrote: > The logic is a bit reversed. The default should be to select the signed > variant, and if the TSS is unsigned, then convert it to the unsigned variant. Using getCorrespondingUnsignedType(). Also changed getCorrespondingUnsignedType() to accept fixed point types. Repository: rC Clang https://reviews.llvm.org/D46911 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47301: Warning for framework include violation from Headers to PrivateHeaders
bruno updated this revision to Diff 149361. bruno added a comment. Update patch after changes to https://reviews.llvm.org/D47157. Also address some of Volodymyr feedback. https://reviews.llvm.org/D47301 Files: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticLexKinds.td lib/Lex/HeaderSearch.cpp test/Modules/Inputs/framework-public-includes-private/A.framework/Headers/A.h test/Modules/Inputs/framework-public-includes-private/A.framework/Modules/module.modulemap test/Modules/Inputs/framework-public-includes-private/A.framework/Modules/module.private.modulemap test/Modules/Inputs/framework-public-includes-private/A.framework/PrivateHeaders/APriv.h test/Modules/Inputs/framework-public-includes-private/A.framework/PrivateHeaders/APriv2.h test/Modules/Inputs/framework-public-includes-private/a.hmap.json test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.h test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.modulemap test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.private.modulemap test/Modules/Inputs/framework-public-includes-private/flat-header-path/ZPriv.h test/Modules/Inputs/framework-public-includes-private/z.hmap.json test/Modules/Inputs/framework-public-includes-private/z.yaml test/Modules/framework-public-includes-private.m Index: test/Modules/framework-public-includes-private.m === --- /dev/null +++ test/Modules/framework-public-includes-private.m @@ -0,0 +1,37 @@ +// REQUIRES: shell + +// RUN: rm -rf %t +// RUN: mkdir %t + +// RUN: hmaptool write %S/Inputs/framework-public-includes-private/a.hmap.json %t/a.hmap +// RUN: hmaptool write %S/Inputs/framework-public-includes-private/z.hmap.json %t/z.hmap + +// RUN: sed -e "s:TEST_DIR:%S/Inputs/framework-public-includes-private:g" \ +// RUN: %S/Inputs/framework-public-includes-private/z.yaml > %t/z.yaml + +// The output with and without modules should be the same, without modules first. +// RUN: %clang_cc1 \ +// RUN: -iquote %t/z.hmap -iquote %t/a.hmap -ivfsoverlay %t/z.yaml \ +// RUN: -F%S/Inputs/framework-public-includes-private \ +// RUN: -fsyntax-only %s -verify + +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache \ +// RUN: -iquote %t/z.hmap -iquote %t/a.hmap -ivfsoverlay %t/z.yaml \ +// RUN: -F%S/Inputs/framework-public-includes-private \ +// RUN: -fsyntax-only %s \ +// RUN: 2>%t/stderr + +// The same warnings show up when modules is on but -verify doesn't get it +// because they only show up under the module A building context. +// RUN: FileCheck --input-file=%t/stderr %s +// CHECK: public framework header includes private framework header 'A/APriv.h' +// CHECK: public framework header includes private framework header 'A/APriv2.h' +// CHECK: public framework header includes private framework header 'Z/ZPriv.h' + +#import "A.h" + +int bar() { return foo(); } + +// expected-warning@Inputs/framework-public-includes-private/A.framework/Headers/A.h:2{{public framework header includes private framework header 'A/APriv.h'}} +// expected-warning@Inputs/framework-public-includes-private/A.framework/Headers/A.h:3{{public framework header includes private framework header 'A/APriv2.h'}} +// expected-warning@Inputs/framework-public-includes-private/flat-header-path/Z.h:1{{public framework header includes private framework header 'Z/ZPriv.h'}} Index: test/Modules/Inputs/framework-public-includes-private/z.yaml === --- /dev/null +++ test/Modules/Inputs/framework-public-includes-private/z.yaml @@ -0,0 +1,45 @@ +{ + 'version': 0, + 'case-sensitive': 'false', + 'use-external-names' : 'false', + 'roots': [ +{ + 'type': 'directory', + 'name': "TEST_DIR/Z.framework/Headers", + 'contents': [ +{ + 'type': 'file', + 'name': "Z.h", + 'external-contents': "TEST_DIR/flat-header-path/Z.h" +} + ] +}, +{ + 'type': 'directory', + 'name': "TEST_DIR/Z.framework/PrivateHeaders", + 'contents': [ +{ + 'type': 'file', + 'name': "ZPriv.h", + 'external-contents': "TEST_DIR/flat-header-path/ZPriv.h" +} + ] +}, +{ + 'type': 'directory', + 'name': "TEST_DIR/Z.framework/Modules", + 'contents': [ +{ + 'type': 'file', + 'name': "module.modulemap", + 'external-contents': "TEST_DIR/flat-header-path/Z.modulemap" +}, +{ + 'type': 'file', + 'name': "module.private.modulemap", + 'external-contents': "TEST_DIR/flat-header-path/Z.private.modulemap" +} + ] +} + ] +} Index: test/Modules/Inputs/framework-public-includes-private/z.hmap.json === --- /dev/null +++ test/Modules/Inputs/fr
[PATCH] D47394: [OpenMP][Clang][NVPTX] Replace bundling with partial linking for the OpenMP NVPTX device offloading toolchain
gtbercea added a comment. > Assuming we do proceed with back-to-CUDA approach, one thing I'd consider > would be using clang's -fcuda-include-gpubinary option which CUDA uses to > include GPU code into the host object. You may be able to use it to avoid > compiling and partially linking .fatbin and host .o. I tried this example (https://devblogs.nvidia.com/separate-compilation-linking-cuda-device-code/). It worked with NVCC but not with clang++. I can produce the main.o particle.o and v.o objects as relocatable (-fcuda-rdc) but the final step fails with a missing reference error. This leads me to believe that embedding the CUDA fatbin code in the host object comes with limitations. If I were to change the OpenMP NVPTX toolchain to do the same then I would run into similar problems. On the other hand., the example, ported to use OpenMP declare target regions (instead of __device__) it all compiles, links and runs correctly. In general, I feel that if we go the way you propose then the solution is truly confined to NVPTX. If we instead implement a scheme like the one in this patch then we give other toolchains a chance to perhaps fill the nvlink "gap" and eventually be able to handle offloading in a similar manner and support static linking. Repository: rC Clang https://reviews.llvm.org/D47394 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47301: Warning for framework include violation from Headers to PrivateHeaders
bruno marked an inline comment as done. bruno added inline comments. Comment at: include/clang/Basic/DiagnosticGroups.td:34-35 def AutoImport : DiagGroup<"auto-import">; def FrameworkHdrQuotedInclude : DiagGroup<"quoted-include-in-framework-header">; +def FrameworkIncludePrivateFromPublic : DiagGroup<"framework-include-private-from-public">; def CXX14BinaryLiteral : DiagGroup<"c++14-binary-literal">; vsapsai wrote: > It might be convenient for users to have a warning group that will cover > different framework warnings, something like -Wframework-hygiene. But it's > out of scope for this review, more as an idea for future improvements. Yep, that should come once we land all other bits. Comment at: lib/Lex/HeaderSearch.cpp:625 +static bool isFrameworkStylePath(StringRef Path, bool &IsPrivateHeader, + SmallVectorImpl &FrameworkName) { using namespace llvm::sys; vsapsai wrote: > In this function we accept some paths that aren't valid framework paths. Need > to think more if it is OK or if we want to be stricter. It should be ok at this point, otherwise the framework style path would have failed before finding this header. Comment at: test/Modules/framework-public-includes-private.m:33 + +int bar() { return foo(); } + vsapsai wrote: > I'm not entirely sure it's not covered by existing test. It might be worth > testing private header including public header and private header including > another private header. The warning is on by default and clang already have the scenario you described in other module tests - those would fail if there's a bug in the logic here. https://reviews.llvm.org/D47301 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47607: [libcxx] Almost fix some UB in and
erik.pilkington created this revision. erik.pilkington added reviewers: rsmith, EricWF, mclow.lists. Herald added subscribers: christof, kosarev. and define `__value_type` as a union between pair and pair so that various operations can move into/from these pairs [1]. This is a pretty blatant strict aliasing violation, and has the potential break code that was optimized by TBAA. We can't remove this, not only would that be a significant pessimization, but we are also required to provide a non-const reference to the key_type to implement splicing maps & sets. This patch fixes `__value_type` by downgrading it from "accidentally launch the missiles"-level UB to theoretical UB, namely, instead of type-punning between pairs we just const_cast the key. It's still undefined to mutate a const object, but clang doesn't actually optimize on this rule. Just to be paranoid this patch also `launder()`s to "pick-up" the new value of key whenever we read from it, though this isn't launder's actual use-case. Thanks to @rsmith for the suggestions! To do this, this patch: - removes the __nc_value_type data member of the `__value_type` union - provides access to the pair with `pair`, which is used to assign through to the pair - changes `__value_type.__cc` to `__value_type.__get_value()`, the latter calls std::launder in C++1z mode. https://reviews.llvm.org/D46845 depends on this patch. [1]: https://stackoverflow.com/a/31667355 Thanks for taking a look! Erik Repository: rCXX libc++ https://reviews.llvm.org/D47607 Files: libcxx/include/__hash_table libcxx/include/__tree libcxx/include/map libcxx/include/unordered_map Index: libcxx/include/unordered_map === --- libcxx/include/unordered_map +++ libcxx/include/unordered_map @@ -396,7 +396,7 @@ const _Hash& hash_function() const _NOEXCEPT {return *this;} _LIBCPP_INLINE_VISIBILITY size_t operator()(const _Cp& __x) const -{return static_cast(*this)(__x.__cc.first);} +{return static_cast(*this)(__x.__get_value().first);} _LIBCPP_INLINE_VISIBILITY size_t operator()(const _Key& __x) const {return static_cast(*this)(__x);} @@ -425,7 +425,7 @@ const _Hash& hash_function() const _NOEXCEPT {return __hash_;} _LIBCPP_INLINE_VISIBILITY size_t operator()(const _Cp& __x) const -{return __hash_(__x.__cc.first);} +{return __hash_(__x.__get_value().first);} _LIBCPP_INLINE_VISIBILITY size_t operator()(const _Key& __x) const {return __hash_(__x);} @@ -464,13 +464,13 @@ const _Pred& key_eq() const _NOEXCEPT {return *this;} _LIBCPP_INLINE_VISIBILITY bool operator()(const _Cp& __x, const _Cp& __y) const -{return static_cast(*this)(__x.__cc.first, __y.__cc.first);} +{return static_cast(*this)(__x.__get_value().first, __y.__get_value().first);} _LIBCPP_INLINE_VISIBILITY bool operator()(const _Cp& __x, const _Key& __y) const -{return static_cast(*this)(__x.__cc.first, __y);} +{return static_cast(*this)(__x.__get_value().first, __y);} _LIBCPP_INLINE_VISIBILITY bool operator()(const _Key& __x, const _Cp& __y) const -{return static_cast(*this)(__x, __y.__cc.first);} +{return static_cast(*this)(__x, __y.__get_value().first);} void swap(__unordered_map_equal&__y) _NOEXCEPT_(__is_nothrow_swappable<_Pred>::value) { @@ -496,13 +496,13 @@ const _Pred& key_eq() const _NOEXCEPT {return __pred_;} _LIBCPP_INLINE_VISIBILITY bool operator()(const _Cp& __x, const _Cp& __y) const -{return __pred_(__x.__cc.first, __y.__cc.first);} +{return __pred_(__x.__get_value().first, __y.__get_value().first);} _LIBCPP_INLINE_VISIBILITY bool operator()(const _Cp& __x, const _Key& __y) const -{return __pred_(__x.__cc.first, __y);} +{return __pred_(__x.__get_value().first, __y);} _LIBCPP_INLINE_VISIBILITY bool operator()(const _Key& __x, const _Cp& __y) const -{return __pred_(__x, __y.__cc.first);} +{return __pred_(__x, __y.__get_value().first);} void swap(__unordered_map_equal&__y) _NOEXCEPT_(__is_nothrow_swappable<_Pred>::value) { @@ -572,9 +572,9 @@ void operator()(pointer __p) _NOEXCEPT { if (__second_constructed) -__alloc_traits::destroy(__na_, _VSTD::addressof(__p->__value_.__cc.second)); +__alloc_traits::destroy(__na_, _VSTD::addressof(__p->__value_.__get_value().second)); if (__first_constructed) -__alloc_traits::destroy(__na_, _VSTD::addressof(__p->__value_.__cc.first)); +__alloc_traits::destroy(__na_, _VSTD::addressof(__p->__value_.__get_value().first)); if (__p) __alloc_traits::deallocate(__na_, __p, 1); } @@ -587,27 +587,69 @@ typedef _Key key_type; typedef _Tp
[PATCH] D44931: [WebAssembly] Use Windows EH instructions for Wasm EH
This revision was automatically updated to reflect the committed changes. Closed by commit rL333703: [WebAssembly] Use Windows EH instructions for Wasm EH (authored by aheejin, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D44931 Files: cfe/trunk/lib/CodeGen/CGCXXABI.h cfe/trunk/lib/CodeGen/CGCleanup.cpp cfe/trunk/lib/CodeGen/CGCleanup.h cfe/trunk/lib/CodeGen/CGException.cpp cfe/trunk/lib/CodeGen/CodeGenFunction.h cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp cfe/trunk/test/CodeGenCXX/wasm-eh.cpp Index: cfe/trunk/test/CodeGenCXX/wasm-eh.cpp === --- cfe/trunk/test/CodeGenCXX/wasm-eh.cpp +++ cfe/trunk/test/CodeGenCXX/wasm-eh.cpp @@ -0,0 +1,384 @@ +// RUN: %clang_cc1 %s -triple wasm32-unknown-unknown -fms-extensions -fexceptions -fcxx-exceptions -emit-llvm -o - -std=c++11 | FileCheck %s +// RUN: %clang_cc1 %s -triple wasm64-unknown-unknown -fms-extensions -fexceptions -fcxx-exceptions -emit-llvm -o - -std=c++11 | FileCheck %s + +void may_throw(); +void dont_throw() noexcept; + +struct Cleanup { + ~Cleanup() { dont_throw(); } +}; + +// Multiple catch clauses w/o catch-all +void test0() { + try { +may_throw(); + } catch (int) { +dont_throw(); + } catch (double) { +dont_throw(); + } +} + +// CHECK-LABEL: define void @_Z5test0v() {{.*}} personality i8* bitcast (i32 (...)* @__gxx_wasm_personality_v0 to i8*) + +// CHECK: %[[INT_ALLOCA:.*]] = alloca i32 +// CHECK: invoke void @_Z9may_throwv() +// CHECK-NEXT: to label %[[NORMAL_BB:.*]] unwind label %[[CATCHDISPATCH_BB:.*]] + +// CHECK: [[CATCHDISPATCH_BB]]: +// CHECK-NEXT: %[[CATCHSWITCH:.*]] = catchswitch within none [label %[[CATCHSTART_BB:.*]]] unwind to caller + +// CHECK: [[CATCHSTART_BB]]: +// CHECK-NEXT: %[[CATCHPAD:.*]] = catchpad within %[[CATCHSWITCH]] [i8* bitcast (i8** @_ZTIi to i8*), i8* bitcast (i8** @_ZTId to i8*)] +// CHECK-NEXT: %[[EXN:.*]] = call i8* @llvm.wasm.get.exception(token %[[CATCHPAD]]) +// CHECK-NEXT: store i8* %[[EXN]], i8** %exn.slot +// CHECK-NEXT: %[[SELECTOR:.*]] = call i32 @llvm.wasm.get.ehselector(token %[[CATCHPAD]]) +// CHECK-NEXT: %[[TYPEID:.*]] = call i32 @llvm.eh.typeid.for(i8* bitcast (i8** @_ZTIi to i8*)) #2 +// CHECK-NEXT: %[[MATCHES:.*]] = icmp eq i32 %[[SELECTOR]], %[[TYPEID]] +// CHECK-NEXT: br i1 %[[MATCHES]], label %[[CATCH_INT_BB:.*]], label %[[CATCH_FALLTHROUGH_BB:.*]] + +// CHECK: [[CATCH_INT_BB]]: +// CHECK-NEXT: %[[EXN:.*]] = load i8*, i8** %exn.slot +// CHECK-NEXT: %[[ADDR:.*]] = call i8* @__cxa_begin_catch(i8* %[[EXN]]) {{.*}} [ "funclet"(token %[[CATCHPAD]]) ] +// CHECK-NEXT: %[[ADDR_CAST:.*]] = bitcast i8* %[[ADDR]] to i32* +// CHECK-NEXT: %[[INT_VAL:.*]] = load i32, i32* %[[ADDR_CAST]] +// CHECK-NEXT: store i32 %[[INT_VAL]], i32* %[[INT_ALLOCA]] +// CHECK-NEXT: call void @_Z10dont_throwv() {{.*}} [ "funclet"(token %[[CATCHPAD]]) ] +// CHECK-NEXT: call void @__cxa_end_catch() {{.*}} [ "funclet"(token %[[CATCHPAD]]) ] +// CHECK-NEXT: catchret from %[[CATCHPAD]] to label %[[CATCHRET_DEST_BB0:.*]] + +// CHECK: [[CATCHRET_DEST_BB0]]: +// CHECK-NEXT: br label %[[TRY_CONT_BB:.*]] + +// CHECK: [[CATCH_FALLTHROUGH_BB]] +// CHECK-NEXT: %[[TYPEID:.*]] = call i32 @llvm.eh.typeid.for(i8* bitcast (i8** @_ZTId to i8*)) #2 +// CHECK-NEXT: %[[MATCHES:.*]] = icmp eq i32 %[[SELECTOR]], %[[TYPEID]] +// CHECK-NEXT: br i1 %[[MATCHES]], label %[[CATCH_FLOAT_BB:.*]], label %[[RETHROW_BB:.*]] + +// CHECK: [[CATCH_FLOAT_BB]]: +// CHECK: catchret from %[[CATCHPAD]] to label %[[CATCHRET_DEST_BB1:.*]] + +// CHECK: [[CATCHRET_DEST_BB1]]: +// CHECK-NEXT: br label %[[TRY_CONT_BB]] + +// CHECK: [[RETHROW_BB]]: +// CHECK-NEXT: call void @__cxa_rethrow() {{.*}} [ "funclet"(token %[[CATCHPAD]]) ] +// CHECK-NEXT: unreachable + +// Single catch-all +void test1() { + try { +may_throw(); + } catch (...) { +dont_throw(); + } +} + +// CATCH-LABEL: @_Z5test1v() + +// CHECK: %[[CATCHSWITCH:.*]] = catchswitch within none [label %[[CATCHSTART_BB:.*]]] unwind to caller + +// CHECK: [[CATCHSTART_BB]]: +// CHECK-NEXT: %[[CATCHPAD:.*]] = catchpad within %[[CATCHSWITCH]] [i8* null] +// CHECK: br label %[[CATCH_ALL_BB:.*]] + +// CHECK: [[CATCH_ALL_BB]]: +// CHECK: catchret from %[[CATCHPAD]] to label + +// Multiple catch clauses w/ catch-all +void test2() { + try { +may_throw(); + } catch (int) { +dont_throw(); + } catch (...) { +dont_throw(); + } +} + +// CHECK-LABEL: @_Z5test2v() + +// CHECK: %[[CATCHSWITCH:.*]] = catchswitch within none [label %[[CATCHSTART_BB:.*]]] unwind to caller + +// CHECK: [[CATCHSTART_BB]]: +// CHECK-NEXT: %[[CATCHPAD:.*]] = catchpad within %[[CATCHSWITCH]] [i8* bitcast (i8** @_ZTIi to i8*), i8* null] +// CHECK: br i1 %{{.*}}, label %[[CATCH_INT_BB:.*]], label %[[CATCH_ALL_BB:.*]] + +// CHECK: [[CATCH_INT_BB]]: +// CHECK: catchret from %[[
r333703 - [WebAssembly] Use Windows EH instructions for Wasm EH
Author: aheejin Date: Thu May 31 15:18:13 2018 New Revision: 333703 URL: http://llvm.org/viewvc/llvm-project?rev=333703&view=rev Log: [WebAssembly] Use Windows EH instructions for Wasm EH Summary: Because wasm control flow needs to be structured, using WinEH instructions to support wasm EH brings several benefits. This patch makes wasm EH uses Windows EH instructions, with some changes: 1. Because wasm uses a single catch block to catch all C++ exceptions, this merges all catch clauses into a single catchpad, within which we test the EH selector as in Itanium EH. 2. Generates a call to `__clang_call_terminate` in case a cleanup throws. Wasm does not have a runtime to handle this. 3. In case there is no catch-all clause, inserts a call to `__cxa_rethrow` at the end of a catchpad in order to unwind to an enclosing EH scope. Reviewers: majnemer, dschuff Subscribers: jfb, sbc100, jgravelle-google, sunfish, cfe-commits Differential Revision: https://reviews.llvm.org/D44931 Added: cfe/trunk/test/CodeGenCXX/wasm-eh.cpp Modified: cfe/trunk/lib/CodeGen/CGCXXABI.h cfe/trunk/lib/CodeGen/CGCleanup.cpp cfe/trunk/lib/CodeGen/CGCleanup.h cfe/trunk/lib/CodeGen/CGException.cpp cfe/trunk/lib/CodeGen/CodeGenFunction.h cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp Modified: cfe/trunk/lib/CodeGen/CGCXXABI.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCXXABI.h?rev=333703&r1=333702&r2=333703&view=diff == --- cfe/trunk/lib/CodeGen/CGCXXABI.h (original) +++ cfe/trunk/lib/CodeGen/CGCXXABI.h Thu May 31 15:18:13 2018 @@ -607,6 +607,17 @@ CGCXXABI *CreateItaniumCXXABI(CodeGenMod /// Creates a Microsoft-family ABI. CGCXXABI *CreateMicrosoftCXXABI(CodeGenModule &CGM); +struct CatchRetScope final : EHScopeStack::Cleanup { + llvm::CatchPadInst *CPI; + + CatchRetScope(llvm::CatchPadInst *CPI) : CPI(CPI) {} + + void Emit(CodeGenFunction &CGF, Flags flags) override { +llvm::BasicBlock *BB = CGF.createBasicBlock("catchret.dest"); +CGF.Builder.CreateCatchRet(CPI, BB); +CGF.EmitBlock(BB); + } +}; } } Modified: cfe/trunk/lib/CodeGen/CGCleanup.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCleanup.cpp?rev=333703&r1=333702&r2=333703&view=diff == --- cfe/trunk/lib/CodeGen/CGCleanup.cpp (original) +++ cfe/trunk/lib/CodeGen/CGCleanup.cpp Thu May 31 15:18:13 2018 @@ -971,16 +971,21 @@ void CodeGenFunction::PopCleanupBlock(bo SaveAndRestore RestoreCurrentFuncletPad( CurrentFuncletPad); llvm::CleanupPadInst *CPI = nullptr; -if (!EHPersonality::get(*this).usesFuncletPads()) { - EHStack.pushTerminate(); - PushedTerminate = true; -} else { + +const EHPersonality &Personality = EHPersonality::get(*this); +if (Personality.usesFuncletPads()) { llvm::Value *ParentPad = CurrentFuncletPad; if (!ParentPad) ParentPad = llvm::ConstantTokenNone::get(CGM.getLLVMContext()); CurrentFuncletPad = CPI = Builder.CreateCleanupPad(ParentPad); } +// Non-MSVC personalities need to terminate when an EH cleanup throws. +if (!Personality.isMSVCPersonality()) { + EHStack.pushTerminate(); + PushedTerminate = true; +} + // We only actually emit the cleanup code if the cleanup is either // active or was used before it was deactivated. if (EHActiveFlag.isValid() || IsActive) { Modified: cfe/trunk/lib/CodeGen/CGCleanup.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCleanup.h?rev=333703&r1=333702&r2=333703&view=diff == --- cfe/trunk/lib/CodeGen/CGCleanup.h (original) +++ cfe/trunk/lib/CodeGen/CGCleanup.h Thu May 31 15:18:13 2018 @@ -627,16 +627,21 @@ struct EHPersonality { static const EHPersonality MSVC_except_handler; static const EHPersonality MSVC_C_specific_handler; static const EHPersonality MSVC_CxxFrameHandler3; + static const EHPersonality GNU_Wasm_CPlusPlus; /// Does this personality use landingpads or the family of pad instructions /// designed to form funclets? - bool usesFuncletPads() const { return isMSVCPersonality(); } + bool usesFuncletPads() const { +return isMSVCPersonality() || isWasmPersonality(); + } bool isMSVCPersonality() const { return this == &MSVC_except_handler || this == &MSVC_C_specific_handler || this == &MSVC_CxxFrameHandler3; } + bool isWasmPersonality() const { return this == &GNU_Wasm_CPlusPlus; } + bool isMSVCXXPersonality() const { return this == &MSVC_CxxFrameHandler3; } }; } Modified: cfe/trunk/lib/CodeGen/CGException.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGException.cpp?rev=333703&r1=333702&r2=333703&view=diff ==
[PATCH] D47394: [OpenMP][Clang][NVPTX] Replace bundling with partial linking for the OpenMP NVPTX device offloading toolchain
tra added a comment. In https://reviews.llvm.org/D47394#1118223, @gtbercea wrote: > I tried this example > (https://devblogs.nvidia.com/separate-compilation-linking-cuda-device-code/). > It worked with NVCC but not with clang++. I can produce the main.o particle.o > and v.o objects as relocatable (-fcuda-rdc) but the final step fails with a > missing reference error. It's not clear what exactly you mean by the "final step" and what exactly was the error. Could you give me more details? > This leads me to believe that embedding the CUDA fatbin code in the host > object comes with limitations. If I were to change the OpenMP NVPTX toolchain > to do the same then I would run into similar problems. It's a two-part problem. In the end, we need to place GPU-side binary (whether it's an object or an executable) in a way that CUDA tools can recognize. You should end up with pretty much the same set of bits. If clang currently does not do that well enough, we should fix it. Second part is what do we do about GPU-side object files. NVCC has some under-the-hood magic that invokes nvlink. If we invoke clang for the final linking phase, it has no idea that some of .o files may have GPU code in it that may need extra steps before we can pass everything to the linker to produce the host executable. IMO the linking of GPU-side objects should be done outside of clang. I.e. one could do it with an extra build rule which would invoke `nvcc --device-link ...` to link all GPU-side objects into a GPU executable, still wrapped in a host .o, which can then be linked into the host executable. > On the other hand., the example, ported to use OpenMP declare target regions > (instead of __device__) it all compiles, links and runs correctly. > > In general, I feel that if we go the way you propose then the solution is > truly confined to NVPTX. If we instead implement a scheme like the one in > this patch then we give other toolchains a chance to perhaps fill the nvlink > "gap" and eventually be able to handle offloading in a similar manner and > support static linking. I'm not sure how is "fatbin + clang -fcuda-gpubinary" is any more confining to NVPTX than "fatbin + clang + ld -r" -- either way you rely on nvidia-specific tool. If at some point you find it too confining, changing either of those will require pretty much the same amount of work. Repository: rC Clang https://reviews.llvm.org/D47394 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47603: [analyzer] fix bug with 1-bit APSInt types in Z3ConstraintManager
george.karpenkov accepted this revision. george.karpenkov added a comment. This revision is now accepted and ready to land. Thanks! Repository: rC Clang https://reviews.llvm.org/D47603 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits