[PATCH] D55681: [llvm] API for encoding/decoding DWARF discriminators.

[Mircea Trofin via Phabricator via llvm-commits]

mtrofin added inline comments.


================
Comment at: lib/IR/DebugInfoMetadata.cpp:152-155
+  BD = getUnsignedFromPrefixEncoding(D);
+  DF = getUnsignedFromPrefixEncoding(getNextComponentInDiscriminator(D));
+  CI = getUnsignedFromPrefixEncoding(
+      getNextComponentInDiscriminator(getNextComponentInDiscriminator(D)));
----------------
dblaikie wrote:
> Perhaps these should call getBaseDiscriminatorFromDiscriminator, getDuplicationFactorFromDiscriminator, getCopyIdentifierFromDiscriminator?
> 
> (though it looks like getDuplicationFactorFromdiscriminator has some extra logic that isn't in the initialization for DF here in decodeDiscriminator - is that a bug that these two cases are handled differently?)
Yes, the current APIs are a bit special-casing. I decided to leave them as-is, and just offer decodeDiscriminator as access to the raw data.


================
Comment at: lib/Target/X86/X86DiscriminateMemOps.cpp:138
         Changed = true;
         const std::pair<DenseSet<unsigned>::iterator, bool> MustInsert =
             Set.insert(DI->getBaseDiscriminator());
----------------
dblaikie wrote:
> I'd probably drop the top level 'const' here (since it's a value rather than a reference - LLVM doesn't usually use top level const like this)
I wanted to make sure no mutation happens through the iterator. I'll remove it for consistency though.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55681