[PATCH] D156571: [DebugInfo] Alternate MD5 fix, NFC

[Paul Robinson via Phabricator via cfe-commits]

probinson added inline comments.



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:423
+  if (!CSInfo) {
+SmallString<64> Checksum;
+std::optional CSKind =

In the final commit, `Checksum` is outside the `if` so that its lifetime 
persists to the end of the function (specifically, past the `createFile` call). 
`CSInfo` holds a StringRef to this string, not the string itself.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156571

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

[PATCH] D156571: [DebugInfo] Alternate MD5 fix, NFC

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

Thanks @DavidSpickett the patch is currently reverted. I have a revised patch 
coming soon and I will keep a close eye on the bots.
I believe it's a string-lifetime issue and so whether it manifests is 
unpredictable, but we have enough different bots in the farm that it did get 
caught.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156571

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

[PATCH] D156571: [DebugInfo] Alternate MD5 fix, NFC

[David Spickett via Phabricator via cfe-commits]

DavidSpickett added a comment.

Failing on one of our 2 stage AArch64 bots 
https://lab.llvm.org/buildbot/#/builders/176/builds/4267 after the reland (that 
bot has been red for various reasons today, so no emails got sent).

Maybe it helps to know that it doesn't fail on the same config with single 
stage https://lab.llvm.org/buildbot/#/builders/197/builds/8998. So it's at 
least able to build simple test cases (the test suite is failing to build for a 
separate reason).

If this is somehow AArch64 only we (Linaro) can help investigate, I see that it 
was ok on PowerPC for example 
https://lab.llvm.org/buildbot/#/builders/36/builds/36728.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156571

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

[PATCH] D156571: [DebugInfo] Alternate MD5 fix, NFC

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

This patch is possibly a suspect in at least some bot failures 
 although I'm at a 
loss to understand why. Perhaps I can't just blithely call getChecksum() and 
copy what it sends back? The ways of metadata remain mysterious to me. I'll be 
poking at this more as time permits, unless someone can tell me my obvious 
mistake.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156571

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

[PATCH] D156571: [DebugInfo] Alternate MD5 fix, NFC

[Nikita Popov via Phabricator via cfe-commits]

nikic added a comment.

Thanks! Can confirm that this recovers the compile-time regression.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156571

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

[PATCH] D156571: [DebugInfo] Alternate MD5 fix, NFC

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

Detail added in the commit message, good idea!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156571

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

[PATCH] D156571: [DebugInfo] Alternate MD5 fix, NFC

[Paul Robinson 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 rGca1295c5a15f: [DebugInfo] Alternate (more efficient) MD5 fix 
(authored by probinson).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156571

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/test/CodeGenCXX/debug-info-function-context.cpp


Index: clang/test/CodeGenCXX/debug-info-function-context.cpp
===
--- clang/test/CodeGenCXX/debug-info-function-context.cpp
+++ clang/test/CodeGenCXX/debug-info-function-context.cpp
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -emit-llvm -debug-info-kind=limited -triple 
x86_64-pc-linux-gnu %s \
-// RUN: -dwarf-version=5 -main-file-name %s  -o - | FileCheck %s
+// RUN: -dwarf-version=5 -main-file-name debug-info-function-context.cpp  
-o - | FileCheck %s
 
 struct C {
   void member_function();
@@ -31,8 +31,8 @@
 
 // The first DIFile is for the CU, the second is what everything else uses.
 // We're using DWARF v5 so both should have MD5 checksums.
-// CHECK: !DIFile(filename: "{{.*}}context.cpp",{{.*}} checksumkind: CSK_MD5
-// CHECK: ![[FILE:[0-9]+]] = !DIFile(filename: "{{.*}}context.cpp",{{.*}} 
checksumkind: CSK_MD5
+// CHECK: !DIFile(filename: "{{.*}}context.cpp",{{.*}} checksumkind: CSK_MD5, 
checksum: [[CKSUM:".*"]]
+// CHECK: ![[FILE:[0-9]+]] = !DIFile(filename: "{{.*}}context.cpp",{{.*}} 
checksumkind: CSK_MD5, checksum: [[CKSUM]]
 // CHECK: ![[C:[0-9]+]] = distinct !DICompositeType(tag: 
DW_TAG_structure_type, name: "C",
 // CHECK: ![[NS:.*]] = !DINamespace(name: "ns"
 // CHECK: !DISubprogram(name: "member_function",{{.*}} scope: ![[C]],{{.*}} 
DISPFlagDefinition
Index: clang/lib/CodeGen/CGDebugInfo.h
===
--- clang/lib/CodeGen/CGDebugInfo.h
+++ clang/lib/CodeGen/CGDebugInfo.h
@@ -148,7 +148,7 @@
   llvm::BumpPtrAllocator DebugInfoNames;
   StringRef CWDName;
 
-  llvm::StringMap DIFileCache;
+  llvm::DenseMap DIFileCache;
   llvm::DenseMap SPCache;
   /// Cache declarations relevant to DW_TAG_imported_declarations (C++
   /// using declarations and global alias variables) that aren't covered
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -391,12 +391,14 @@
   SourceManager &SM = CGM.getContext().getSourceManager();
   StringRef FileName;
   FileID FID;
+  std::optional> CSInfo;
 
   if (Loc.isInvalid()) {
 // The DIFile used by the CU is distinct from the main source file. Call
 // createFile() below for canonicalization if the source file was specified
 // with an absolute path.
 FileName = TheCU->getFile()->getFilename();
+CSInfo = TheCU->getFile()->getChecksum();
   } else {
 PresumedLoc PLoc = SM.getPresumedLoc(Loc);
 FileName = PLoc.getFilename();
@@ -417,13 +419,13 @@
   return cast(V);
   }
 
-  SmallString<64> Checksum;
-
-  std::optional CSKind =
+  if (!CSInfo) {
+SmallString<64> Checksum;
+std::optional CSKind =
   computeChecksum(FID, Checksum);
-  std::optional> CSInfo;
-  if (CSKind)
-CSInfo.emplace(*CSKind, Checksum);
+if (CSKind)
+  CSInfo.emplace(*CSKind, Checksum);
+  }
   return createFile(FileName, CSInfo, getSource(SM, SM.getFileID(Loc)));
 }
 


Index: clang/test/CodeGenCXX/debug-info-function-context.cpp
===
--- clang/test/CodeGenCXX/debug-info-function-context.cpp
+++ clang/test/CodeGenCXX/debug-info-function-context.cpp
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -emit-llvm -debug-info-kind=limited -triple x86_64-pc-linux-gnu %s \
-// RUN: -dwarf-version=5 -main-file-name %s  -o - | FileCheck %s
+// RUN: -dwarf-version=5 -main-file-name debug-info-function-context.cpp  -o - | FileCheck %s
 
 struct C {
   void member_function();
@@ -31,8 +31,8 @@
 
 // The first DIFile is for the CU, the second is what everything else uses.
 // We're using DWARF v5 so both should have MD5 checksums.
-// CHECK: !DIFile(filename: "{{.*}}context.cpp",{{.*}} checksumkind: CSK_MD5
-// CHECK: ![[FILE:[0-9]+]] = !DIFile(filename: "{{.*}}context.cpp",{{.*}} checksumkind: CSK_MD5
+// CHECK: !DIFile(filename: "{{.*}}context.cpp",{{.*}} checksumkind: CSK_MD5, checksum: [[CKSUM:".*"]]
+// CHECK: ![[FILE:[0-9]+]] = !DIFile(filename: "{{.*}}context.cpp",{{.*}} checksumkind: CSK_MD5, checksum: [[CKSUM]]
 // CHECK: ![[C:[0-9]+]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "C",
 // CHECK: ![[NS:.*]] = !DINamespace(name: "ns"
 // CHECK: !DISubprogram(name: "member_function",{{.*}} scope: ![[C]],{{.*}} DISPFlagDefinition
Index: clang/lib/CodeGen/CGDebugInfo.h
=

[PATCH] D156571: [DebugInfo] Alternate MD5 fix, NFC

[David Blaikie via Phabricator via cfe-commits]

dblaikie accepted this revision.
dblaikie added a comment.
This revision is now accepted and ready to land.

Sounds OK to me - thanks!

bit more detail in the actual patch description/commit message (from some of 
the comments in this review, perhaps) would be handy.


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

https://reviews.llvm.org/D156571

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

[PATCH] D156571: [DebugInfo] Alternate MD5 fix, NFC

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

Ping


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

https://reviews.llvm.org/D156571

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

[PATCH] D156571: [DebugInfo] Alternate MD5 fix, NFC

[Paul Robinson via Phabricator via cfe-commits]

probinson updated this revision to Diff 546144.
probinson added a comment.

Reuse the main file's checksum instead of recalculating it.


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

https://reviews.llvm.org/D156571

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/test/CodeGenCXX/debug-info-function-context.cpp


Index: clang/test/CodeGenCXX/debug-info-function-context.cpp
===
--- clang/test/CodeGenCXX/debug-info-function-context.cpp
+++ clang/test/CodeGenCXX/debug-info-function-context.cpp
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -emit-llvm -debug-info-kind=limited -triple 
x86_64-pc-linux-gnu %s \
-// RUN: -dwarf-version=5 -main-file-name %s  -o - | FileCheck %s
+// RUN: -dwarf-version=5 -main-file-name debug-info-function-context.cpp  
-o - | FileCheck %s
 
 struct C {
   void member_function();
@@ -31,8 +31,8 @@
 
 // The first DIFile is for the CU, the second is what everything else uses.
 // We're using DWARF v5 so both should have MD5 checksums.
-// CHECK: !DIFile(filename: "{{.*}}context.cpp",{{.*}} checksumkind: CSK_MD5
-// CHECK: ![[FILE:[0-9]+]] = !DIFile(filename: "{{.*}}context.cpp",{{.*}} 
checksumkind: CSK_MD5
+// CHECK: !DIFile(filename: "{{.*}}context.cpp",{{.*}} checksumkind: CSK_MD5, 
checksum: [[CKSUM:".*"]]
+// CHECK: ![[FILE:[0-9]+]] = !DIFile(filename: "{{.*}}context.cpp",{{.*}} 
checksumkind: CSK_MD5, checksum: [[CKSUM]]
 // CHECK: ![[C:[0-9]+]] = distinct !DICompositeType(tag: 
DW_TAG_structure_type, name: "C",
 // CHECK: ![[NS:.*]] = !DINamespace(name: "ns"
 // CHECK: !DISubprogram(name: "member_function",{{.*}} scope: ![[C]],{{.*}} 
DISPFlagDefinition
Index: clang/lib/CodeGen/CGDebugInfo.h
===
--- clang/lib/CodeGen/CGDebugInfo.h
+++ clang/lib/CodeGen/CGDebugInfo.h
@@ -148,7 +148,7 @@
   llvm::BumpPtrAllocator DebugInfoNames;
   StringRef CWDName;
 
-  llvm::StringMap DIFileCache;
+  llvm::DenseMap DIFileCache;
   llvm::DenseMap SPCache;
   /// Cache declarations relevant to DW_TAG_imported_declarations (C++
   /// using declarations and global alias variables) that aren't covered
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -391,12 +391,14 @@
   SourceManager &SM = CGM.getContext().getSourceManager();
   StringRef FileName;
   FileID FID;
+  std::optional> CSInfo;
 
   if (Loc.isInvalid()) {
 // The DIFile used by the CU is distinct from the main source file. Call
 // createFile() below for canonicalization if the source file was specified
 // with an absolute path.
 FileName = TheCU->getFile()->getFilename();
+CSInfo = TheCU->getFile()->getChecksum();
   } else {
 PresumedLoc PLoc = SM.getPresumedLoc(Loc);
 FileName = PLoc.getFilename();
@@ -417,13 +419,13 @@
   return cast(V);
   }
 
-  SmallString<64> Checksum;
-
-  std::optional CSKind =
+  if (!CSInfo) {
+SmallString<64> Checksum;
+std::optional CSKind =
   computeChecksum(FID, Checksum);
-  std::optional> CSInfo;
-  if (CSKind)
-CSInfo.emplace(*CSKind, Checksum);
+if (CSKind)
+  CSInfo.emplace(*CSKind, Checksum);
+  }
   return createFile(FileName, CSInfo, getSource(SM, SM.getFileID(Loc)));
 }
 


Index: clang/test/CodeGenCXX/debug-info-function-context.cpp
===
--- clang/test/CodeGenCXX/debug-info-function-context.cpp
+++ clang/test/CodeGenCXX/debug-info-function-context.cpp
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -emit-llvm -debug-info-kind=limited -triple x86_64-pc-linux-gnu %s \
-// RUN: -dwarf-version=5 -main-file-name %s  -o - | FileCheck %s
+// RUN: -dwarf-version=5 -main-file-name debug-info-function-context.cpp  -o - | FileCheck %s
 
 struct C {
   void member_function();
@@ -31,8 +31,8 @@
 
 // The first DIFile is for the CU, the second is what everything else uses.
 // We're using DWARF v5 so both should have MD5 checksums.
-// CHECK: !DIFile(filename: "{{.*}}context.cpp",{{.*}} checksumkind: CSK_MD5
-// CHECK: ![[FILE:[0-9]+]] = !DIFile(filename: "{{.*}}context.cpp",{{.*}} checksumkind: CSK_MD5
+// CHECK: !DIFile(filename: "{{.*}}context.cpp",{{.*}} checksumkind: CSK_MD5, checksum: [[CKSUM:".*"]]
+// CHECK: ![[FILE:[0-9]+]] = !DIFile(filename: "{{.*}}context.cpp",{{.*}} checksumkind: CSK_MD5, checksum: [[CKSUM]]
 // CHECK: ![[C:[0-9]+]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "C",
 // CHECK: ![[NS:.*]] = !DINamespace(name: "ns"
 // CHECK: !DISubprogram(name: "member_function",{{.*}} scope: ![[C]],{{.*}} DISPFlagDefinition
Index: clang/lib/CodeGen/CGDebugInfo.h
===
--- clang/lib/CodeGen/CGDebugInfo.h
+++ clang/lib/CodeGen/CGDebugInfo.h
@@ -148,7 +148,7 @@
   llvm::BumpPtrAllocator 

[PATCH] D156571: [DebugInfo] Alternate MD5 fix, NFC

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

Of course this means we're rerunning MD5 on the main source file; probably can 
capture that from TheCU and save that cost as well.


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

https://reviews.llvm.org/D156571

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

[PATCH] D156571: [DebugInfo] Alternate MD5 fix, NFC

[Paul Robinson via Phabricator via cfe-commits]

probinson updated this revision to Diff 546115.
probinson added a comment.

Use the main FileID instead of expensive string compares.

Figured this out after staring at CreateCompileUnit for long enough. Seeding 
the DIFileCache with the DIFile created there made another test unhappy 
(difile_entry.cpp) but this fix doesn't.


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

https://reviews.llvm.org/D156571

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/test/CodeGenCXX/debug-info-function-context.cpp


Index: clang/test/CodeGenCXX/debug-info-function-context.cpp
===
--- clang/test/CodeGenCXX/debug-info-function-context.cpp
+++ clang/test/CodeGenCXX/debug-info-function-context.cpp
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -emit-llvm -debug-info-kind=limited -triple 
x86_64-pc-linux-gnu %s \
-// RUN: -dwarf-version=5 -main-file-name %s  -o - | FileCheck %s
+// RUN: -dwarf-version=5 -main-file-name debug-info-function-context.cpp  
-o - | FileCheck %s
 
 struct C {
   void member_function();
Index: clang/lib/CodeGen/CGDebugInfo.h
===
--- clang/lib/CodeGen/CGDebugInfo.h
+++ clang/lib/CodeGen/CGDebugInfo.h
@@ -148,7 +148,7 @@
   llvm::BumpPtrAllocator DebugInfoNames;
   StringRef CWDName;
 
-  llvm::StringMap DIFileCache;
+  llvm::DenseMap DIFileCache;
   llvm::DenseMap SPCache;
   /// Cache declarations relevant to DW_TAG_imported_declarations (C++
   /// using declarations and global alias variables) that aren't covered
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -397,6 +397,7 @@
 // createFile() below for canonicalization if the source file was specified
 // with an absolute path.
 FileName = TheCU->getFile()->getFilename();
+FID = SM.getMainFileID();
   } else {
 PresumedLoc PLoc = SM.getPresumedLoc(Loc);
 FileName = PLoc.getFilename();


Index: clang/test/CodeGenCXX/debug-info-function-context.cpp
===
--- clang/test/CodeGenCXX/debug-info-function-context.cpp
+++ clang/test/CodeGenCXX/debug-info-function-context.cpp
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -emit-llvm -debug-info-kind=limited -triple x86_64-pc-linux-gnu %s \
-// RUN: -dwarf-version=5 -main-file-name %s  -o - | FileCheck %s
+// RUN: -dwarf-version=5 -main-file-name debug-info-function-context.cpp  -o - | FileCheck %s
 
 struct C {
   void member_function();
Index: clang/lib/CodeGen/CGDebugInfo.h
===
--- clang/lib/CodeGen/CGDebugInfo.h
+++ clang/lib/CodeGen/CGDebugInfo.h
@@ -148,7 +148,7 @@
   llvm::BumpPtrAllocator DebugInfoNames;
   StringRef CWDName;
 
-  llvm::StringMap DIFileCache;
+  llvm::DenseMap DIFileCache;
   llvm::DenseMap SPCache;
   /// Cache declarations relevant to DW_TAG_imported_declarations (C++
   /// using declarations and global alias variables) that aren't covered
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -397,6 +397,7 @@
 // createFile() below for canonicalization if the source file was specified
 // with an absolute path.
 FileName = TheCU->getFile()->getFilename();
+FID = SM.getMainFileID();
   } else {
 PresumedLoc PLoc = SM.getPresumedLoc(Loc);
 FileName = PLoc.getFilename();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156571: [DebugInfo] Alternate MD5 fix, NFC

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

The CU's DIFile is conjured up in CGDebugInfo::CreateCompileUnit(), and the 
name is derived from `-main-file-name` rather than anything SourceManager 
provides. Although there is a comment wondering about that. CreateCompileUnit() 
computes a checksum for that DIFile, but apparently the DIFile never makes it 
into the DIFileCache.

DIFileCache is generated by CGDebugInfo::createFile(); it looks like there are 
some subtle differences in the path handling between what it does and what 
CreateCompileUnit() does, so making CreateCompileUnit() go through createFile() 
might not the the right thing. But it does look like CreateCompileUnit() might 
just be missing a one-liner to add the CU's DIFile to DIFileCache.

I'll try that instead.


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

https://reviews.llvm.org/D156571

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

[PATCH] D156571: [DebugInfo] Alternate MD5 fix, NFC

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

From the other review:

>> Could/should we do the lookup on the CU filename before it goes into the DI 
>> metadata, and store that FileID somewhere for later use?
>
> There's a note in issue 63955 to the effect that I can't find an API to turn 
> a filename into a FileID. If there is one that I didn't find, we could use 
> FileIDs instead of pointers to name strings.

I was thinking more along the lines of keeping the same code we have, string 
pointer equality (though perhaps FileID is equivalent in cases where the 
pointer euqality is valid, and the FileID approach would be more self 
documenting/less subtle). I was thinking - for other files we get filenames 
correctly that are pointer identical because they're in the FileID, right? But 
for this one because we stash it in/get it from the DICompileUnit, that 
identity gets lost. But presumably the identity is true for the string filename 
used to build the DICompileUnit? So if, at that point, we stored the pointer, 
or FileID aside and used that instead of retrieving it from the DICompileUnit, 
perhaps that'd make this name consistent with the other names in being able to 
use pointer identity?


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

https://reviews.llvm.org/D156571

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

[PATCH] D156571: [DebugInfo] Alternate MD5 fix, NFC

[Paul Robinson via Phabricator via cfe-commits]

probinson created this revision.
probinson added reviewers: dblaikie, nikic.
probinson added a project: debug-info.
Herald added a subscriber: StephenFan.
Herald added a project: All.
probinson requested review of this revision.

Should have lower memory and time cost than D155991 
.


https://reviews.llvm.org/D156571

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDebugInfo.h


Index: clang/lib/CodeGen/CGDebugInfo.h
===
--- clang/lib/CodeGen/CGDebugInfo.h
+++ clang/lib/CodeGen/CGDebugInfo.h
@@ -148,7 +148,7 @@
   llvm::BumpPtrAllocator DebugInfoNames;
   StringRef CWDName;
 
-  llvm::StringMap DIFileCache;
+  llvm::DenseMap DIFileCache;
   llvm::DenseMap SPCache;
   /// Cache declarations relevant to DW_TAG_imported_declarations (C++
   /// using declarations and global alias variables) that aren't covered
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -397,6 +397,17 @@
 // createFile() below for canonicalization if the source file was specified
 // with an absolute path.
 FileName = TheCU->getFile()->getFilename();
+for (auto It : DIFileCache) {
+  // StringRef operator== does a content comparison.
+  if (FileName == It.first) {
+// Got a string match, use the existing DIFile if possible.
+if (llvm::Metadata *V = It.second)
+  return cast(V);
+// Fall through to create the DIFile but use the original string.
+FileName = It.first;
+break;
+  }
+}
   } else {
 PresumedLoc PLoc = SM.getPresumedLoc(Loc);
 FileName = PLoc.getFilename();


Index: clang/lib/CodeGen/CGDebugInfo.h
===
--- clang/lib/CodeGen/CGDebugInfo.h
+++ clang/lib/CodeGen/CGDebugInfo.h
@@ -148,7 +148,7 @@
   llvm::BumpPtrAllocator DebugInfoNames;
   StringRef CWDName;
 
-  llvm::StringMap DIFileCache;
+  llvm::DenseMap DIFileCache;
   llvm::DenseMap SPCache;
   /// Cache declarations relevant to DW_TAG_imported_declarations (C++
   /// using declarations and global alias variables) that aren't covered
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -397,6 +397,17 @@
 // createFile() below for canonicalization if the source file was specified
 // with an absolute path.
 FileName = TheCU->getFile()->getFilename();
+for (auto It : DIFileCache) {
+  // StringRef operator== does a content comparison.
+  if (FileName == It.first) {
+// Got a string match, use the existing DIFile if possible.
+if (llvm::Metadata *V = It.second)
+  return cast(V);
+// Fall through to create the DIFile but use the original string.
+FileName = It.first;
+break;
+  }
+}
   } else {
 PresumedLoc PLoc = SM.getPresumedLoc(Loc);
 FileName = PLoc.getFilename();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits