Thanks Duncan! On 15 April 2017 at 01:25, Duncan P. N. Exon Smith <dexonsm...@apple.com> wrote:
> FYI, I reverted the modules side of this in r300380. For details, see the > commit message. TL;DR: this didn't actually make modules builds closer to > matching non-modules builds, thanks to how submodules work; on the > contrary, it made them diverge. > > > On 2017-Mar-31, at 08:36, Alex Lorenz via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > > > > Author: arphaman > > Date: Fri Mar 31 10:36:21 2017 > > New Revision: 299226 > > > > URL: http://llvm.org/viewvc/llvm-project?rev=299226&view=rev > > Log: > > [Modules][PCH] Serialize #pragma pack > > > > This patch serializes the state of #pragma pack. It preserves the state > of the > > pragma from a PCH/from modules in a file that uses that PCH/those > modules. > > > > > rdar://21359084 > > > > Differential Revision: https://reviews.llvm.org/D31241 > > > > Added: > > cfe/trunk/test/Modules/Inputs/pragma_pack_push.h > > cfe/trunk/test/Modules/Inputs/pragma_pack_reset_push.h > > cfe/trunk/test/Modules/Inputs/pragma_pack_set.h > > cfe/trunk/test/Modules/pragma-pack.c > > cfe/trunk/test/PCH/pragma-pack.c > > Modified: > > cfe/trunk/include/clang/Serialization/ASTBitCodes.h > > cfe/trunk/include/clang/Serialization/ASTReader.h > > cfe/trunk/include/clang/Serialization/ASTWriter.h > > cfe/trunk/lib/Sema/SemaAttr.cpp > > cfe/trunk/lib/Serialization/ASTReader.cpp > > cfe/trunk/lib/Serialization/ASTWriter.cpp > > cfe/trunk/test/Modules/Inputs/module.map > > > > Modified: cfe/trunk/include/clang/Serialization/ASTBitCodes.h > > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/ > clang/Serialization/ASTBitCodes.h?rev=299226&r1=299225&r2=299226&view=diff > > ============================================================ > ================== > > --- cfe/trunk/include/clang/Serialization/ASTBitCodes.h (original) > > +++ cfe/trunk/include/clang/Serialization/ASTBitCodes.h Fri Mar 31 > 10:36:21 2017 > > @@ -604,6 +604,9 @@ namespace clang { > > OPENCL_EXTENSION_DECLS = 59, > > > > MODULAR_CODEGEN_DECLS = 60, > > + > > + /// \brief Record code for \#pragma pack options. > > + PACK_PRAGMA_OPTIONS = 61, > > }; > > > > /// \brief Record types used within a source manager block. > > > > Modified: cfe/trunk/include/clang/Serialization/ASTReader.h > > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/ > clang/Serialization/ASTReader.h?rev=299226&r1=299225&r2=299226&view=diff > > ============================================================ > ================== > > --- cfe/trunk/include/clang/Serialization/ASTReader.h (original) > > +++ cfe/trunk/include/clang/Serialization/ASTReader.h Fri Mar 31 > 10:36:21 2017 > > @@ -811,6 +811,17 @@ private: > > int PragmaMSPointersToMembersState = -1; > > SourceLocation PointersToMembersPragmaLocation; > > > > + /// \brief The pragma pack state. > > + Optional<unsigned> PragmaPackCurrentValue; > > + SourceLocation PragmaPackCurrentLocation; > > + struct PragmaPackStackEntry { > > + unsigned Value; > > + SourceLocation Location; > > + StringRef SlotLabel; > > + }; > > + llvm::SmallVector<PragmaPackStackEntry, 2> PragmaPackStack; > > + llvm::SmallVector<std::string, 2> PragmaPackStrings; > > + > > /// \brief The OpenCL extension settings. > > OpenCLOptions OpenCLExtensions; > > > > > > Modified: cfe/trunk/include/clang/Serialization/ASTWriter.h > > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/ > clang/Serialization/ASTWriter.h?rev=299226&r1=299225&r2=299226&view=diff > > ============================================================ > ================== > > --- cfe/trunk/include/clang/Serialization/ASTWriter.h (original) > > +++ cfe/trunk/include/clang/Serialization/ASTWriter.h Fri Mar 31 > 10:36:21 2017 > > @@ -485,6 +485,7 @@ private: > > void WriteOptimizePragmaOptions(Sema &SemaRef); > > void WriteMSStructPragmaOptions(Sema &SemaRef); > > void WriteMSPointersToMembersPragmaOptions(Sema &SemaRef); > > + void WritePackPragmaOptions(Sema &SemaRef); > > void WriteModuleFileExtension(Sema &SemaRef, > > ModuleFileExtensionWriter &Writer); > > > > > > Modified: cfe/trunk/lib/Sema/SemaAttr.cpp > > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/ > SemaAttr.cpp?rev=299226&r1=299225&r2=299226&view=diff > > ============================================================ > ================== > > --- cfe/trunk/lib/Sema/SemaAttr.cpp (original) > > +++ cfe/trunk/lib/Sema/SemaAttr.cpp Fri Mar 31 10:36:21 2017 > > @@ -215,6 +215,7 @@ void Sema::PragmaStack<ValueType>::Act(S > > ValueType Value) { > > if (Action == PSK_Reset) { > > CurrentValue = DefaultValue; > > + CurrentPragmaLocation = PragmaLocation; > > return; > > } > > if (Action & PSK_Push) > > > > Modified: cfe/trunk/lib/Serialization/ASTReader.cpp > > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ > Serialization/ASTReader.cpp?rev=299226&r1=299225&r2=299226&view=diff > > ============================================================ > ================== > > --- cfe/trunk/lib/Serialization/ASTReader.cpp (original) > > +++ cfe/trunk/lib/Serialization/ASTReader.cpp Fri Mar 31 10:36:21 2017 > > @@ -3298,6 +3298,28 @@ ASTReader::ReadASTBlock(ModuleFile &F, u > > } > > ForceCUDAHostDeviceDepth = Record[0]; > > break; > > + > > + case PACK_PRAGMA_OPTIONS: { > > + if (Record.size() < 3) { > > + Error("invalid pragma pack record"); > > + return Failure; > > + } > > + PragmaPackCurrentValue = Record[0]; > > + PragmaPackCurrentLocation = ReadSourceLocation(F, Record[1]); > > + unsigned NumStackEntries = Record[2]; > > + unsigned Idx = 3; > > + // Reset the stack when importing a new module. > > + PragmaPackStack.clear(); > > + for (unsigned I = 0; I < NumStackEntries; ++I) { > > + PragmaPackStackEntry Entry; > > + Entry.Value = Record[Idx++]; > > + Entry.Location = ReadSourceLocation(F, Record[Idx++]); > > + PragmaPackStrings.push_back(ReadString(Record, Idx)); > > + Entry.SlotLabel = PragmaPackStrings.back(); > > + PragmaPackStack.push_back(Entry); > > + } > > + break; > > + } > > } > > } > > } > > @@ -7419,6 +7441,34 @@ void ASTReader::UpdateSema() { > > PointersToMembersPragmaLocation); > > } > > SemaObj->ForceCUDAHostDeviceDepth = ForceCUDAHostDeviceDepth; > > + > > + if (PragmaPackCurrentValue) { > > + // The bottom of the stack might have a default value. It must be > adjusted > > + // to the current value to ensure that the packing state is > preserved after > > + // popping entries that were included/imported from a PCH/module. > > + bool DropFirst = false; > > + if (!PragmaPackStack.empty() && > > + PragmaPackStack.front().Location.isInvalid()) { > > + assert(PragmaPackStack.front().Value == > > SemaObj->PackStack.DefaultValue > && > > + "Expected a default alignment value"); > > + SemaObj->PackStack.Stack.emplace_back( > > + PragmaPackStack.front().SlotLabel, SemaObj->PackStack. > CurrentValue, > > + SemaObj->PackStack.CurrentPragmaLocation); > > + DropFirst = true; > > + } > > + for (const auto &Entry : > > + llvm::makeArrayRef(PragmaPackStack).drop_front(DropFirst ? 1 > : 0)) > > + SemaObj->PackStack.Stack.emplace_back(Entry.SlotLabel, > Entry.Value, > > + Entry.Location); > > + if (PragmaPackCurrentLocation.isInvalid()) { > > + assert(*PragmaPackCurrentValue == SemaObj->PackStack.DefaultValue > && > > + "Expected a default alignment value"); > > + // Keep the current values. > > + } else { > > + SemaObj->PackStack.CurrentValue = *PragmaPackCurrentValue; > > + SemaObj->PackStack.CurrentPragmaLocation = > PragmaPackCurrentLocation; > > + } > > + } > > } > > > > IdentifierInfo *ASTReader::get(StringRef Name) { > > > > Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp > > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ > Serialization/ASTWriter.cpp?rev=299226&r1=299225&r2=299226&view=diff > > ============================================================ > ================== > > --- cfe/trunk/lib/Serialization/ASTWriter.cpp (original) > > +++ cfe/trunk/lib/Serialization/ASTWriter.cpp Fri Mar 31 10:36:21 2017 > > @@ -4170,6 +4170,20 @@ void ASTWriter::WriteMSPointersToMembers > > Stream.EmitRecord(POINTERS_TO_MEMBERS_PRAGMA_OPTIONS, Record); > > } > > > > +/// \brief Write the state of 'pragma pack' at the end of the module. > > +void ASTWriter::WritePackPragmaOptions(Sema &SemaRef) { > > + RecordData Record; > > + Record.push_back(SemaRef.PackStack.CurrentValue); > > + AddSourceLocation(SemaRef.PackStack.CurrentPragmaLocation, Record); > > + Record.push_back(SemaRef.PackStack.Stack.size()); > > + for (const auto &StackEntry : SemaRef.PackStack.Stack) { > > + Record.push_back(StackEntry.Value); > > + AddSourceLocation(StackEntry.PragmaLocation, Record); > > + AddString(StackEntry.StackSlotLabel, Record); > > + } > > + Stream.EmitRecord(PACK_PRAGMA_OPTIONS, Record); > > +} > > + > > void ASTWriter::WriteModuleFileExtension(Sema &SemaRef, > > ModuleFileExtensionWriter > &Writer) { > > // Enter the extension block. > > @@ -4860,6 +4874,7 @@ ASTFileSignature ASTWriter::WriteASTCore > > WriteMSStructPragmaOptions(SemaRef); > > WriteMSPointersToMembersPragmaOptions(SemaRef); > > } > > + WritePackPragmaOptions(SemaRef); > > > > // Some simple statistics > > RecordData::value_type Record[] = { > > > > Modified: cfe/trunk/test/Modules/Inputs/module.map > > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ > Modules/Inputs/module.map?rev=299226&r1=299225&r2=299226&view=diff > > ============================================================ > ================== > > --- cfe/trunk/test/Modules/Inputs/module.map (original) > > +++ cfe/trunk/test/Modules/Inputs/module.map Fri Mar 31 10:36:21 2017 > > @@ -265,6 +265,22 @@ module diag_pragma { > > header "diag_pragma.h" > > } > > > > +module pragma_pack_set { > > + header "pragma_pack_set.h" > > +} > > + > > +module pragma_pack_push { > > + header "pragma_pack_push.h" > > +} > > + > > +module pragma_pack_empty { > > + header "empty.h" > > +} > > + > > +module pragma_pack_reset_push { > > + header "pragma_pack_reset_push.h" > > +} > > + > > module dummy { > > header "dummy.h" > > } > > > > Added: cfe/trunk/test/Modules/Inputs/pragma_pack_push.h > > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ > Modules/Inputs/pragma_pack_push.h?rev=299226&view=auto > > ============================================================ > ================== > > --- cfe/trunk/test/Modules/Inputs/pragma_pack_push.h (added) > > +++ cfe/trunk/test/Modules/Inputs/pragma_pack_push.h Fri Mar 31 > 10:36:21 2017 > > @@ -0,0 +1,6 @@ > > + > > +#pragma pack (push, 4) > > +#pragma pack (push, 2) > > +#pragma pack (push, 1) > > +#pragma pack (pop) > > + > > > > Added: cfe/trunk/test/Modules/Inputs/pragma_pack_reset_push.h > > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ > Modules/Inputs/pragma_pack_reset_push.h?rev=299226&view=auto > > ============================================================ > ================== > > --- cfe/trunk/test/Modules/Inputs/pragma_pack_reset_push.h (added) > > +++ cfe/trunk/test/Modules/Inputs/pragma_pack_reset_push.h Fri Mar 31 > 10:36:21 2017 > > @@ -0,0 +1,4 @@ > > + > > +#pragma pack () > > +#pragma pack (push, 4) > > + > > > > Added: cfe/trunk/test/Modules/Inputs/pragma_pack_set.h > > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ > Modules/Inputs/pragma_pack_set.h?rev=299226&view=auto > > ============================================================ > ================== > > --- cfe/trunk/test/Modules/Inputs/pragma_pack_set.h (added) > > +++ cfe/trunk/test/Modules/Inputs/pragma_pack_set.h Fri Mar 31 10:36:21 > 2017 > > @@ -0,0 +1,3 @@ > > + > > +#pragma pack (1) > > + > > > > Added: cfe/trunk/test/Modules/pragma-pack.c > > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ > Modules/pragma-pack.c?rev=299226&view=auto > > ============================================================ > ================== > > --- cfe/trunk/test/Modules/pragma-pack.c (added) > > +++ cfe/trunk/test/Modules/pragma-pack.c Fri Mar 31 10:36:21 2017 > > @@ -0,0 +1,35 @@ > > +// RUN: rm -rf %t > > +// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fmodules > -fimplicit-module-maps -x objective-c -emit-module -fmodules-cache-path=%t > -fmodule-name=pragma_pack_set %S/Inputs/module.map > > +// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fmodules > -fimplicit-module-maps -x objective-c -emit-module -fmodules-cache-path=%t > -fmodule-name=pragma_pack_push %S/Inputs/module.map > > +// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fmodules > -fimplicit-module-maps -x objective-c -emit-module -fmodules-cache-path=%t > -fmodule-name=pragma_pack_empty %S/Inputs/module.map > > +// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fmodules > -fimplicit-module-maps -x objective-c -emit-module -fmodules-cache-path=%t > -fmodule-name=pragma_pack_reset_push %S/Inputs/module.map > > +// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fmodules > -fimplicit-module-maps -x objective-c -verify -fmodules-cache-path=%t -I > %S/Inputs %s > > +// FIXME: When we have a syntax for modules in C, use that. > > + > > +@import pragma_pack_set; > > + > > +#pragma pack (show) // expected-warning {{value of #pragma pack(show) > == 1}} > > +#pragma pack (pop) // expected-warning {{#pragma pack(pop, ...) failed: > stack empty}} > > + > > +@import pragma_pack_push; > > + > > +#pragma pack (show) // expected-warning {{value of #pragma pack(show) > == 2}} > > +#pragma pack (pop) > > +#pragma pack (show) // expected-warning {{value of #pragma pack(show) > == 4}} > > +#pragma pack (pop) > > +#pragma pack (show) // expected-warning {{value of #pragma pack(show) > == 1}} > > +#pragma pack (pop) // expected-warning {{#pragma pack(pop, ...) failed: > stack empty}} > > + > > +#pragma pack (16) > > + > > +@import pragma_pack_empty; > > + > > +#pragma pack (show) // expected-warning {{value of #pragma pack(show) > == 16}} > > +#pragma pack (pop) // expected-warning {{#pragma pack(pop, ...) failed: > stack empty}} > > + > > +@import pragma_pack_reset_push; > > + > > +#pragma pack (show) // expected-warning {{value of #pragma pack(show) > == 4}} > > +#pragma pack (pop) > > +#pragma pack (show) // expected-warning {{value of #pragma pack(show) > == 8}} > > + > > > > Added: cfe/trunk/test/PCH/pragma-pack.c > > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/PCH/ > pragma-pack.c?rev=299226&view=auto > > ============================================================ > ================== > > --- cfe/trunk/test/PCH/pragma-pack.c (added) > > +++ cfe/trunk/test/PCH/pragma-pack.c Fri Mar 31 10:36:21 2017 > > @@ -0,0 +1,90 @@ > > +// Test this without pch. > > +// RUN: %clang_cc1 -triple x86_64-apple-darwin10 %s -include %s -verify > -fsyntax-only -DSET > > +// RUN: %clang_cc1 -triple x86_64-apple-darwin10 %s -include %s -verify > -fsyntax-only -DRESET > > +// RUN: %clang_cc1 -triple x86_64-apple-darwin10 %s -include %s -verify > -fsyntax-only -DPUSH > > +// RUN: %clang_cc1 -triple x86_64-apple-darwin10 %s -include %s -verify > -fsyntax-only -DPUSH_POP > > +// RUN: %clang_cc1 -triple x86_64-apple-darwin10 %s -include %s -verify > -fsyntax-only -DPUSH_POP_LABEL > > + > > +// Test with pch. > > +// RUN: %clang_cc1 -triple x86_64-apple-darwin10 %s -DSET -emit-pch -o > %t > > +// RUN: %clang_cc1 -triple x86_64-apple-darwin10 %s -DSET -verify > -include-pch %t > > +// RUN: %clang_cc1 -triple x86_64-apple-darwin10 %s -DRESET -emit-pch > -o %t > > +// RUN: %clang_cc1 -triple x86_64-apple-darwin10 %s -DRESET -verify > -include-pch %t > > +// RUN: %clang_cc1 -triple x86_64-apple-darwin10 %s -DPUSH -emit-pch -o > %t > > +// RUN: %clang_cc1 -triple x86_64-apple-darwin10 %s -DPUSH -verify > -include-pch %t > > +// RUN: %clang_cc1 -triple x86_64-apple-darwin10 %s -DPUSH_POP > -emit-pch -o %t > > +// RUN: %clang_cc1 -triple x86_64-apple-darwin10 %s -DPUSH_POP -verify > -include-pch %t > > +// RUN: %clang_cc1 -triple x86_64-apple-darwin10 %s -DPUSH_POP_LABEL > -emit-pch -o %t > > +// RUN: %clang_cc1 -triple x86_64-apple-darwin10 %s -DPUSH_POP_LABEL > -verify -include-pch %t > > + > > +#ifndef HEADER > > +#define HEADER > > + > > +#ifdef SET > > +#pragma pack(1) > > +#endif > > + > > +#ifdef RESET > > +#pragma pack(2) > > +#pragma pack () > > +#endif > > + > > +#ifdef PUSH > > +#pragma pack(1) > > +#pragma pack (push, 2) > > +#endif > > + > > +#ifdef PUSH_POP > > +#pragma pack (push, 4) > > +#pragma pack (push, 2) > > +#pragma pack (pop) > > +#endif > > + > > +#ifdef PUSH_POP_LABEL > > +#pragma pack (push, a, 4) > > +#pragma pack (push, b, 1) > > +#pragma pack (push, c, 2) > > +#pragma pack (pop, b) > > +#endif > > + > > +#else > > + > > +#ifdef SET > > +#pragma pack(show) // expected-warning {{value of #pragma pack(show) == > 1}} > > +#pragma pack(pop) // expected-warning {{#pragma pack(pop, ...) failed: > stack empty}} > > +#endif > > + > > +#ifdef RESET > > +#pragma pack(show) // expected-warning {{value of #pragma pack(show) == > 8}} > > +#pragma () > > +#pragma pack(show) // expected-warning {{value of #pragma pack(show) == > 8}} > > +#endif > > + > > +#ifdef PUSH > > +#pragma pack(show) // expected-warning {{value of #pragma pack(show) == > 2}} > > +#pragma pack(pop) > > +#pragma pack(show) // expected-warning {{value of #pragma pack(show) == > 1}} > > +#pragma pack () > > +#pragma pack (show) // expected-warning {{value of #pragma pack(show) > == 8}} > > +#pragma pack(pop) // expected-warning {{#pragma pack(pop, ...) failed: > stack empty}} > > +#endif > > + > > +#ifdef PUSH_POP > > +#pragma pack(show) // expected-warning {{value of #pragma pack(show) == > 4}} > > +#pragma pack(pop) > > +#pragma pack(show) // expected-warning {{value of #pragma pack(show) == > 8}} > > +#pragma pack(pop) // expected-warning {{#pragma pack(pop, ...) failed: > stack empty}} > > +#endif > > + > > +#ifdef PUSH_POP_LABEL > > +#pragma pack(show) // expected-warning {{value of #pragma pack(show) == > 4}} > > +#pragma pack(pop, c) > > +#pragma pack(show) // expected-warning {{value of #pragma pack(show) == > 4}} > > +#pragma pack(pop, a) > > +#pragma pack(show) // expected-warning {{value of #pragma pack(show) == > 8}} > > +#pragma pack(pop) // expected-warning {{#pragma pack(pop, ...) failed: > stack empty}} > > +#pragma pack(pop, b) // expected-warning {{#pragma pack(pop, ...) > failed: stack empty}} > > +#pragma pack(show) // expected-warning {{value of #pragma pack(show) == > 8}} > > +#endif > > + > > +#endif > > > > > > _______________________________________________ > > cfe-commits mailing list > > cfe-commits@lists.llvm.org > > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
