[PATCH] D158572: [clang][modules] Use relative offsets for input files

[Petr Hosek via Phabricator via cfe-commits]

phosek added a comment.

In D158572#4615234 , @phosek wrote:

> In D158572#4615231 , @jansvoboda11 
> wrote:
>
>> @phosek BTW can you confirm whether these started failing with this patch or 
>> with D158573 ?
>
> I'm trying to do a local build now to find out.

Looks like it was D158573  so it should be 
safe to reland this, sorry about the confusion.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158572/new/

https://reviews.llvm.org/D158572

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158572: [clang][modules] Use relative offsets for input files

[Petr Hosek via Phabricator via cfe-commits]

phosek added a comment.

In D158572#4615231 , @jansvoboda11 
wrote:

> @phosek BTW can you confirm whether these started failing with this patch or 
> with D158573 ?

I'm trying to do a local build now to find out.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158572/new/

https://reviews.llvm.org/D158572

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158572: [clang][modules] Use relative offsets for input files

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

@phosek BTW can you confirm whether these started failing with this patch or 
with D158573 ?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158572/new/

https://reviews.llvm.org/D158572

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158572: [clang][modules] Use relative offsets for input files

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

In D158572#4615198 , @phosek wrote:

> After this change, all libc++ `clang_modules_include.gen.py` tests started 
> failing on our Linux builders:
>
>   ...
>
> You can see the full output at 
> https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-linux-arm64/b8771862804321535569/test-results.
>  Would it be possible to revert this change?

Thanks for notifying me. Reverting now...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158572/new/

https://reviews.llvm.org/D158572

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158572: [clang][modules] Use relative offsets for input files

[Petr Hosek via Phabricator via cfe-commits]

phosek added a comment.

After this change, all libc++ `clang_modules_include.gen.py` tests started 
failing on our Linux builders:

  Script:
  --
  : 'COMPILED WITH';  /b/s/w/ir/x/w/llvm_build/./bin/clang++ 
/b/s/w/ir/x/w/llvm_build/runtimes/runtimes-aarch64-unknown-linux-gnu-bins/test/libcxx/clang_modules_include.gen.py/algorithm.compile.pass.cpp
 -pthread --target=aarch64-unknown-linux-gnu -nostdinc++ -I 
/b/s/w/ir/x/w/llvm_build/include/c++/v1 -I 
/b/s/w/ir/x/w/llvm_build/include/aarch64-unknown-linux-gnu/c++/v1 -I 
/b/s/w/ir/x/w/llvm-llvm-project/libcxx/test/support -std=c++26 -Werror -Wall 
-Wctad-maybe-unsupported -Wextra -Wshadow -Wundef -Wunused-template 
-Wno-unused-command-line-argument -Wno-attributes -Wno-pessimizing-move 
-Wno-noexcept-type -Wno-atomic-alignment -Wno-reserved-module-identifier 
-Wno-user-defined-literals -Wno-tautological-compare -Wsign-compare 
-Wunused-variable -Wunused-parameter -Wunreachable-code 
-Wno-unused-local-typedef -Wno-local-type-template-args -Wno-c++11-extensions 
-Wno-unknown-pragmas -Wno-pass-failed -Wno-mismatched-new-delete 
-Wno-redundant-move -Wno-self-move -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER 
-D_LIBCPP_ENABLE_EXPERIMENTAL -Werror=thread-safety -Wuser-defined-warnings  
-fsyntax-only
  : 'RUN: at line 2';   /b/s/w/ir/x/w/llvm_build/./bin/clang++ 
/b/s/w/ir/x/w/llvm_build/runtimes/runtimes-aarch64-unknown-linux-gnu-bins/test/libcxx/clang_modules_include.gen.py/algorithm.compile.pass.cpp
 -pthread --target=aarch64-unknown-linux-gnu -nostdinc++ -I 
/b/s/w/ir/x/w/llvm_build/include/c++/v1 -I 
/b/s/w/ir/x/w/llvm_build/include/aarch64-unknown-linux-gnu/c++/v1 -I 
/b/s/w/ir/x/w/llvm-llvm-project/libcxx/test/support -std=c++26 -Werror -Wall 
-Wctad-maybe-unsupported -Wextra -Wshadow -Wundef -Wunused-template 
-Wno-unused-command-line-argument -Wno-attributes -Wno-pessimizing-move 
-Wno-noexcept-type -Wno-atomic-alignment -Wno-reserved-module-identifier 
-Wno-user-defined-literals -Wno-tautological-compare -Wsign-compare 
-Wunused-variable -Wunused-parameter -Wunreachable-code 
-Wno-unused-local-typedef -Wno-local-type-template-args -Wno-c++11-extensions 
-Wno-unknown-pragmas -Wno-pass-failed -Wno-mismatched-new-delete 
-Wno-redundant-move -Wno-self-move -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER 
-D_LIBCPP_ENABLE_EXPERIMENTAL -Werror=thread-safety -Wuser-defined-warnings  
-fmodules -fcxx-modules 
-fmodules-cache-path=/b/s/w/ir/x/w/llvm_build/runtimes/runtimes-aarch64-unknown-linux-gnu-bins/test/libcxx/clang_modules_include.gen.py/Output/algorithm.compile.pass.cpp.dir/t.tmp
 -fsyntax-only
  --
  Exit Code: 1
  
  Command Output (stdout):
  --
  $ ":" "COMPILED WITH"
  $ "/b/s/w/ir/x/w/llvm_build/./bin/clang++" 
"/b/s/w/ir/x/w/llvm_build/runtimes/runtimes-aarch64-unknown-linux-gnu-bins/test/libcxx/clang_modules_include.gen.py/algorithm.compile.pass.cpp"
 "-pthread" "--target=aarch64-unknown-linux-gnu" "-nostdinc++" "-I" 
"/b/s/w/ir/x/w/llvm_build/include/c++/v1" "-I" 
"/b/s/w/ir/x/w/llvm_build/include/aarch64-unknown-linux-gnu/c++/v1" "-I" 
"/b/s/w/ir/x/w/llvm-llvm-project/libcxx/test/support" "-std=c++26" "-Werror" 
"-Wall" "-Wctad-maybe-unsupported" "-Wextra" "-Wshadow" "-Wundef" 
"-Wunused-template" "-Wno-unused-command-line-argument" "-Wno-attributes" 
"-Wno-pessimizing-move" "-Wno-noexcept-type" "-Wno-atomic-alignment" 
"-Wno-reserved-module-identifier" "-Wno-user-defined-literals" 
"-Wno-tautological-compare" "-Wsign-compare" "-Wunused-variable" 
"-Wunused-parameter" "-Wunreachable-code" "-Wno-unused-local-typedef" 
"-Wno-local-type-template-args" "-Wno-c++11-extensions" "-Wno-unknown-pragmas" 
"-Wno-pass-failed" "-Wno-mismatched-new-delete" "-Wno-redundant-move" 
"-Wno-self-move" "-D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER" 
"-D_LIBCPP_ENABLE_EXPERIMENTAL" "-Werror=thread-safety" 
"-Wuser-defined-warnings" "-fsyntax-only"
  $ ":" "RUN: at line 2"
  $ "/b/s/w/ir/x/w/llvm_build/./bin/clang++" 
"/b/s/w/ir/x/w/llvm_build/runtimes/runtimes-aarch64-unknown-linux-gnu-bins/test/libcxx/clang_modules_include.gen.py/algorithm.compile.pass.cpp"
 "-pthread" "--target=aarch64-unknown-linux-gnu" "-nostdinc++" "-I" 
"/b/s/w/ir/x/w/llvm_build/include/c++/v1" "-I" 
"/b/s/w/ir/x/w/llvm_build/include/aarch64-unknown-linux-gnu/c++/v1" "-I" 
"/b/s/w/ir/x/w/llvm-llvm-project/libcxx/test/support" "-std=c++26" "-Werror" 
"-Wall" "-Wctad-maybe-unsupported" "-Wextra" "-Wshadow" "-Wundef" 
"-Wunused-template" "-Wno-unused-command-line-argument" "-Wno-attributes" 
"-Wno-pessimizing-move" "-Wno-noexcept-type" "-Wno-atomic-alignment" 
"-Wno-reserved-module-identifier" "-Wno-user-defined-literals" 
"-Wno-tautological-compare" "-Wsign-compare" "-Wunused-variable" 
"-Wunused-parameter" "-Wunreachable-code" "-Wno-unused-local-typedef" 
"-Wno-local-type-template-args" "-Wno-c++11-extensions" "-Wno-unknown-pragmas" 
"-Wno-pass-failed" "-Wno-mismatched-new-delete" "-Wno-redundant-move" 
"-Wno-self-move" "-D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER" 
"-D_LIBCPP_ENABLE_EXPERIMENTAL" "-Werror=thread-safety" 

[PATCH] D158572: [clang][modules] Use relative offsets for input files

[Jan Svoboda via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb9d78bdc730b: [clang][modules] Use relative offsets for 
input files (authored by jansvoboda11).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158572/new/

https://reviews.llvm.org/D158572

Files:
  clang/include/clang/Serialization/ModuleFile.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp


Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1570,6 +1570,8 @@
   IFHAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 32));
   unsigned IFHAbbrevCode = Stream.EmitAbbrev(std::move(IFHAbbrev));
 
+  uint64_t InputFilesOffsetBase = Stream.GetCurrentBitNo();
+
   // Get all ContentCache objects for files.
   std::vector UserFiles;
   std::vector SystemFiles;
@@ -1633,7 +1635,7 @@
   continue; // already recorded this file.
 
 // Record this entry's offset.
-InputFileOffsets.push_back(Stream.GetCurrentBitNo());
+InputFileOffsets.push_back(Stream.GetCurrentBitNo() - 
InputFilesOffsetBase);
 
 InputFileID = InputFileOffsets.size();
 
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -2326,7 +2326,8 @@
   // Go find this input file.
   BitstreamCursor  = F.InputFilesCursor;
   SavedStreamPosition SavedPosition(Cursor);
-  if (llvm::Error Err = Cursor.JumpToBit(F.InputFileOffsets[ID - 1])) {
+  if (llvm::Error Err = Cursor.JumpToBit(F.InputFilesOffsetBase +
+ F.InputFileOffsets[ID - 1])) {
 // FIXME this drops errors on the floor.
 consumeError(std::move(Err));
   }
@@ -2410,7 +2411,8 @@
   // Go find this input file.
   BitstreamCursor  = F.InputFilesCursor;
   SavedStreamPosition SavedPosition(Cursor);
-  if (llvm::Error Err = Cursor.JumpToBit(F.InputFileOffsets[ID - 1])) {
+  if (llvm::Error Err = Cursor.JumpToBit(F.InputFilesOffsetBase +
+ F.InputFileOffsets[ID - 1])) {
 // FIXME this drops errors on the floor.
 consumeError(std::move(Err));
   }
@@ -2788,6 +2790,7 @@
   Error("malformed block record in AST file");
   return Failure;
 }
+F.InputFilesOffsetBase = F.InputFilesCursor.GetCurrentBitNo();
 continue;
 
   case OPTIONS_BLOCK_ID:
@@ -5328,6 +5331,7 @@
   bool NeedsSystemInputFiles = Listener.needsSystemInputFileVisitation();
   bool NeedsImports = Listener.needsImportVisitation();
   BitstreamCursor InputFilesCursor;
+  uint64_t InputFilesOffsetBase = 0;
 
   RecordData Record;
   std::string ModuleDir;
@@ -5363,6 +5367,7 @@
 if (NeedsInputFiles &&
 ReadBlockAbbrevs(InputFilesCursor, INPUT_FILES_BLOCK_ID))
   return true;
+InputFilesOffsetBase = InputFilesCursor.GetCurrentBitNo();
 break;
 
   default:
@@ -5435,7 +5440,8 @@
 
 BitstreamCursor  = InputFilesCursor;
 SavedStreamPosition SavedPosition(Cursor);
-if (llvm::Error Err = Cursor.JumpToBit(InputFileOffs[I])) {
+if (llvm::Error Err =
+Cursor.JumpToBit(InputFilesOffsetBase + InputFileOffs[I])) {
   // FIXME this drops errors on the floor.
   consumeError(std::move(Err));
 }
Index: clang/include/clang/Serialization/ModuleFile.h
===
--- clang/include/clang/Serialization/ModuleFile.h
+++ clang/include/clang/Serialization/ModuleFile.h
@@ -245,7 +245,10 @@
   /// The cursor to the start of the input-files block.
   llvm::BitstreamCursor InputFilesCursor;
 
-  /// Offsets for all of the input file entries in the AST file.
+  /// Absolute offset of the start of the input-files block.
+  uint64_t InputFilesOffsetBase = 0;
+
+  /// Relative offsets for all of the input file entries in the AST file.
   const llvm::support::unaligned_uint64_t *InputFileOffsets = nullptr;
 
   /// The input files that have been loaded from this AST file.


Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1570,6 +1570,8 @@
   IFHAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 32));
   unsigned IFHAbbrevCode = Stream.EmitAbbrev(std::move(IFHAbbrev));
 
+  uint64_t InputFilesOffsetBase = Stream.GetCurrentBitNo();
+
   // Get all ContentCache objects for files.
   std::vector UserFiles;
   std::vector SystemFiles;
@@ -1633,7 +1635,7 @@
   continue; // already recorded this file.
 
 // Record this entry's offset.
-

[PATCH] D158572: [clang][modules] Use relative offsets for input files

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 updated this revision to Diff 552894.
jansvoboda11 added a comment.

Initialize absolute offsets in `ASTReader`/`ModuleFile`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158572/new/

https://reviews.llvm.org/D158572

Files:
  clang/include/clang/Serialization/ModuleFile.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp


Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1570,6 +1570,8 @@
   IFHAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 32));
   unsigned IFHAbbrevCode = Stream.EmitAbbrev(std::move(IFHAbbrev));
 
+  uint64_t InputFilesOffsetBase = Stream.GetCurrentBitNo();
+
   // Get all ContentCache objects for files.
   std::vector UserFiles;
   std::vector SystemFiles;
@@ -1633,7 +1635,7 @@
   continue; // already recorded this file.
 
 // Record this entry's offset.
-InputFileOffsets.push_back(Stream.GetCurrentBitNo());
+InputFileOffsets.push_back(Stream.GetCurrentBitNo() - 
InputFilesOffsetBase);
 
 InputFileID = InputFileOffsets.size();
 
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -2326,7 +2326,8 @@
   // Go find this input file.
   BitstreamCursor  = F.InputFilesCursor;
   SavedStreamPosition SavedPosition(Cursor);
-  if (llvm::Error Err = Cursor.JumpToBit(F.InputFileOffsets[ID - 1])) {
+  if (llvm::Error Err = Cursor.JumpToBit(F.InputFilesOffsetBase +
+ F.InputFileOffsets[ID - 1])) {
 // FIXME this drops errors on the floor.
 consumeError(std::move(Err));
   }
@@ -2410,7 +2411,8 @@
   // Go find this input file.
   BitstreamCursor  = F.InputFilesCursor;
   SavedStreamPosition SavedPosition(Cursor);
-  if (llvm::Error Err = Cursor.JumpToBit(F.InputFileOffsets[ID - 1])) {
+  if (llvm::Error Err = Cursor.JumpToBit(F.InputFilesOffsetBase +
+ F.InputFileOffsets[ID - 1])) {
 // FIXME this drops errors on the floor.
 consumeError(std::move(Err));
   }
@@ -2788,6 +2790,7 @@
   Error("malformed block record in AST file");
   return Failure;
 }
+F.InputFilesOffsetBase = F.InputFilesCursor.GetCurrentBitNo();
 continue;
 
   case OPTIONS_BLOCK_ID:
@@ -5328,6 +5331,7 @@
   bool NeedsSystemInputFiles = Listener.needsSystemInputFileVisitation();
   bool NeedsImports = Listener.needsImportVisitation();
   BitstreamCursor InputFilesCursor;
+  uint64_t InputFilesOffsetBase = 0;
 
   RecordData Record;
   std::string ModuleDir;
@@ -5363,6 +5367,7 @@
 if (NeedsInputFiles &&
 ReadBlockAbbrevs(InputFilesCursor, INPUT_FILES_BLOCK_ID))
   return true;
+InputFilesOffsetBase = InputFilesCursor.GetCurrentBitNo();
 break;
 
   default:
@@ -5435,7 +5440,8 @@
 
 BitstreamCursor  = InputFilesCursor;
 SavedStreamPosition SavedPosition(Cursor);
-if (llvm::Error Err = Cursor.JumpToBit(InputFileOffs[I])) {
+if (llvm::Error Err =
+Cursor.JumpToBit(InputFilesOffsetBase + InputFileOffs[I])) {
   // FIXME this drops errors on the floor.
   consumeError(std::move(Err));
 }
Index: clang/include/clang/Serialization/ModuleFile.h
===
--- clang/include/clang/Serialization/ModuleFile.h
+++ clang/include/clang/Serialization/ModuleFile.h
@@ -245,7 +245,10 @@
   /// The cursor to the start of the input-files block.
   llvm::BitstreamCursor InputFilesCursor;
 
-  /// Offsets for all of the input file entries in the AST file.
+  /// Absolute offset of the start of the input-files block.
+  uint64_t InputFilesOffsetBase = 0;
+
+  /// Relative offsets for all of the input file entries in the AST file.
   const llvm::support::unaligned_uint64_t *InputFileOffsets = nullptr;
 
   /// The input files that have been loaded from this AST file.


Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1570,6 +1570,8 @@
   IFHAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 32));
   unsigned IFHAbbrevCode = Stream.EmitAbbrev(std::move(IFHAbbrev));
 
+  uint64_t InputFilesOffsetBase = Stream.GetCurrentBitNo();
+
   // Get all ContentCache objects for files.
   std::vector UserFiles;
   std::vector SystemFiles;
@@ -1633,7 +1635,7 @@
   continue; // already recorded this file.
 
 // Record this entry's offset.
-InputFileOffsets.push_back(Stream.GetCurrentBitNo());
+InputFileOffsets.push_back(Stream.GetCurrentBitNo() - InputFilesOffsetBase);
 
 

[PATCH] D158572: [clang][modules] Use relative offsets for input files

[Ben Langmuir via Phabricator via cfe-commits]

benlangmuir added inline comments.



Comment at: clang/include/clang/Serialization/ModuleFile.h:249
+  /// Absolute offset of the start of the input-files block.
+  uint64_t InputFilesOffsetBase;
+

jansvoboda11 wrote:
> benlangmuir wrote:
> > Doesn't `InputFilesCursor` already know where the input files block starts?
> I think it does. We should be able to remove this and always call 
> `InputFilesCursor::GetCurrentBitNo()` at the start. I implemented it this way 
> because it's consistent with the existing pattern: 
> `SLocEntryCursor`/`SourceManagerBlockStartOffset`, 
> `MacroCursor`/`MacroOffsetsBase`, `DeclsCursor`/`DeclsBlockStartOffset`.
> 
> LMK if you feel strongly about it and I can look into getting rid of the 
> extra offset base members.
Since it's consistent with other uses I'm fine with keeping it as-is for this 
patch. Would be nice to clean them all up in a follow up, but not required.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158572/new/

https://reviews.llvm.org/D158572

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158572: [clang][modules] Use relative offsets for input files

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added inline comments.



Comment at: clang/include/clang/Serialization/ModuleFile.h:249
+  /// Absolute offset of the start of the input-files block.
+  uint64_t InputFilesOffsetBase;
+

benlangmuir wrote:
> Doesn't `InputFilesCursor` already know where the input files block starts?
I think it does. We should be able to remove this and always call 
`InputFilesCursor::GetCurrentBitNo()` at the start. I implemented it this way 
because it's consistent with the existing pattern: 
`SLocEntryCursor`/`SourceManagerBlockStartOffset`, 
`MacroCursor`/`MacroOffsetsBase`, `DeclsCursor`/`DeclsBlockStartOffset`.

LMK if you feel strongly about it and I can look into getting rid of the extra 
offset base members.



Comment at: clang/lib/Serialization/ASTReader.cpp:5334
   BitstreamCursor InputFilesCursor;
+  uint64_t InputFilesOffsetBase;
 

benlangmuir wrote:
> We should initialize this to something - either 0 or maybe ~0 so it will be 
> invalid?
Good point, I'll do that along with initializing the member variable.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158572/new/

https://reviews.llvm.org/D158572

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158572: [clang][modules] Use relative offsets for input files

[Ben Langmuir via Phabricator via cfe-commits]

benlangmuir added inline comments.



Comment at: clang/include/clang/Serialization/ModuleFile.h:249
+  /// Absolute offset of the start of the input-files block.
+  uint64_t InputFilesOffsetBase;
+

Doesn't `InputFilesCursor` already know where the input files block starts?



Comment at: clang/lib/Serialization/ASTReader.cpp:5334
   BitstreamCursor InputFilesCursor;
+  uint64_t InputFilesOffsetBase;
 

We should initialize this to something - either 0 or maybe ~0 so it will be 
invalid?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158572/new/

https://reviews.llvm.org/D158572

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158572: [clang][modules] Use relative offsets for input files

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 created this revision.
jansvoboda11 added a reviewer: benlangmuir.
Herald added a subscriber: ributzka.
Herald added a project: All.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch replaces absolute offsets into the input files block with offsets 
relative to the block start. This makes the whole section "relocatable". I 
confirmed all other uses of `GetCurrentBitNo()` are turned into relative 
offsets before being serialized into the AST file.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158572

Files:
  clang/include/clang/Serialization/ModuleFile.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp


Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1570,6 +1570,8 @@
   IFHAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 32));
   unsigned IFHAbbrevCode = Stream.EmitAbbrev(std::move(IFHAbbrev));
 
+  uint64_t InputFilesOffsetBase = Stream.GetCurrentBitNo();
+
   // Get all ContentCache objects for files.
   std::vector UserFiles;
   std::vector SystemFiles;
@@ -1633,7 +1635,7 @@
   continue; // already recorded this file.
 
 // Record this entry's offset.
-InputFileOffsets.push_back(Stream.GetCurrentBitNo());
+InputFileOffsets.push_back(Stream.GetCurrentBitNo() - 
InputFilesOffsetBase);
 
 InputFileID = InputFileOffsets.size();
 
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -2326,7 +2326,8 @@
   // Go find this input file.
   BitstreamCursor  = F.InputFilesCursor;
   SavedStreamPosition SavedPosition(Cursor);
-  if (llvm::Error Err = Cursor.JumpToBit(F.InputFileOffsets[ID - 1])) {
+  if (llvm::Error Err = Cursor.JumpToBit(F.InputFilesOffsetBase +
+ F.InputFileOffsets[ID - 1])) {
 // FIXME this drops errors on the floor.
 consumeError(std::move(Err));
   }
@@ -2410,7 +2411,8 @@
   // Go find this input file.
   BitstreamCursor  = F.InputFilesCursor;
   SavedStreamPosition SavedPosition(Cursor);
-  if (llvm::Error Err = Cursor.JumpToBit(F.InputFileOffsets[ID - 1])) {
+  if (llvm::Error Err = Cursor.JumpToBit(F.InputFilesOffsetBase +
+ F.InputFileOffsets[ID - 1])) {
 // FIXME this drops errors on the floor.
 consumeError(std::move(Err));
   }
@@ -2788,6 +2790,7 @@
   Error("malformed block record in AST file");
   return Failure;
 }
+F.InputFilesOffsetBase = F.InputFilesCursor.GetCurrentBitNo();
 continue;
 
   case OPTIONS_BLOCK_ID:
@@ -5328,6 +5331,7 @@
   bool NeedsSystemInputFiles = Listener.needsSystemInputFileVisitation();
   bool NeedsImports = Listener.needsImportVisitation();
   BitstreamCursor InputFilesCursor;
+  uint64_t InputFilesOffsetBase;
 
   RecordData Record;
   std::string ModuleDir;
@@ -5363,6 +5367,7 @@
 if (NeedsInputFiles &&
 ReadBlockAbbrevs(InputFilesCursor, INPUT_FILES_BLOCK_ID))
   return true;
+InputFilesOffsetBase = InputFilesCursor.GetCurrentBitNo();
 break;
 
   default:
@@ -5435,7 +5440,8 @@
 
 BitstreamCursor  = InputFilesCursor;
 SavedStreamPosition SavedPosition(Cursor);
-if (llvm::Error Err = Cursor.JumpToBit(InputFileOffs[I])) {
+if (llvm::Error Err =
+Cursor.JumpToBit(InputFilesOffsetBase + InputFileOffs[I])) {
   // FIXME this drops errors on the floor.
   consumeError(std::move(Err));
 }
Index: clang/include/clang/Serialization/ModuleFile.h
===
--- clang/include/clang/Serialization/ModuleFile.h
+++ clang/include/clang/Serialization/ModuleFile.h
@@ -245,7 +245,10 @@
   /// The cursor to the start of the input-files block.
   llvm::BitstreamCursor InputFilesCursor;
 
-  /// Offsets for all of the input file entries in the AST file.
+  /// Absolute offset of the start of the input-files block.
+  uint64_t InputFilesOffsetBase;
+
+  /// Relative offsets for all of the input file entries in the AST file.
   const llvm::support::unaligned_uint64_t *InputFileOffsets = nullptr;
 
   /// The input files that have been loaded from this AST file.


Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1570,6 +1570,8 @@
   IFHAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 32));
   unsigned IFHAbbrevCode = Stream.EmitAbbrev(std::move(IFHAbbrev));
 
+  uint64_t InputFilesOffsetBase = Stream.GetCurrentBitNo();
+
   // Get all ContentCache objects