[llvm] Clarify responsiblity of some of DILocation discriminator APIs

Summary:
Renamed setBaseDiscriminator to cloneWithBaseDiscriminator, to match
similar APIs. Also changed its behavior to copy over the other
discriminator components, instead of eliding them.

Renamed cloneWithDuplicationFactor to
cloneByMultiplyingDuplicationFactor, which more closely matches what
this API does.

Reviewers: dblaikie, wmi

Reviewed By: dblaikie

Subscribers: zzheng, llvm-commits

Differential Revision: https://reviews.llvm.org/D56220

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@351996 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/llvm/IR/DebugInfoMetadata.h b/include/llvm/IR/DebugInfoMetadata.h
index 849dc13ba96f1cd0fa3cb82b24ff5ef87e006754..40e6873eba79229f69a073c5e3f3c569132ab554 100644 (file)
--- a/include/llvm/IR/DebugInfoMetadata.h
+++ b/include/llvm/IR/DebugInfoMetadata.h
@@ -1540,9 +1540,6 @@ public:
   ///
   /// For precise control over the data being encoded in the discriminator,
   /// use encodeDiscriminator/decodeDiscriminator.
-  ///
-  /// Use {get|set}BaseDiscriminator and cloneWithDuplicationFactor after reading
-  /// their documentation, as their behavior has side-effects.
 
   inline unsigned getDiscriminator() const;
 
@@ -1553,7 +1550,7 @@ public:
   /// base discriminator is set in the new DILocation, the other encoded values
   /// are elided.
   /// If the discriminator cannot be encoded, the function returns None.
-  inline Optional<const DILocation *> setBaseDiscriminator(unsigned BD) const;
+  inline Optional<const DILocation *> cloneWithBaseDiscriminator(unsigned BD) const;
 
   /// Returns the duplication factor stored in the discriminator, or 1 if no
   /// duplication factor (or 0) is encoded.
@@ -1569,7 +1566,7 @@ public:
   /// duplication factor encoded in the discriminator. The current duplication
   /// factor is as defined by getDuplicationFactor().
   /// Returns None if encoding failed.
-  inline Optional<const DILocation *> cloneWithDuplicationFactor(unsigned DF) const;
+  inline Optional<const DILocation *> cloneByMultiplyingDuplicationFactor(unsigned DF) const;
 
   /// When two instructions are combined into a single instruction we also
   /// need to combine the original locations into a single location.
@@ -1593,10 +1590,11 @@ public:
     return getUnsignedFromPrefixEncoding(D);
   }
 
-  /// Raw encoding of the discriminator. APIs such as setBaseDiscriminator or
-  /// cloneWithDuplicationFactor have certain side-effects. This API, in
-  /// conjunction with cloneWithDiscriminator, may be used to encode precisely
-  /// the values provided. \p BD: base discriminator \p DF: duplication factor
+  /// Raw encoding of the discriminator. APIs such as cloneWithDuplicationFactor
+  /// have certain special case behavior (e.g. treating empty duplication factor
+  /// as the value '1').
+  /// This API, in conjunction with cloneWithDiscriminator, may be used to encode
+  /// the raw values provided. \p BD: base discriminator \p DF: duplication factor
   /// \p CI: copy index
   /// The return is None if the values cannot be encoded in 32 bits - for
   /// example, values for BD or DF larger than 12 bits. Otherwise, the return
@@ -2038,15 +2036,17 @@ unsigned DILocation::getCopyIdentifier() const {
   return getCopyIdentifierFromDiscriminator(getDiscriminator());
 }
 
-Optional<const DILocation *> DILocation::setBaseDiscriminator(unsigned D) const {
-  if (D == 0)
+Optional<const DILocation *> DILocation::cloneWithBaseDiscriminator(unsigned D) const {
+  unsigned BD, DF, CI;
+  decodeDiscriminator(getDiscriminator(), BD, DF, CI);
+  if (D == BD)
     return this;
-  if (D > 0xfff)
-    return None;
-  return cloneWithDiscriminator(encodeComponent(D));
+  if (Optional<unsigned> Encoded = encodeDiscriminator(D, DF, CI))
+    return cloneWithDiscriminator(*Encoded);
+  return None;
 }
 
-Optional<const DILocation *> DILocation::cloneWithDuplicationFactor(unsigned DF) const {
+Optional<const DILocation *> DILocation::cloneByMultiplyingDuplicationFactor(unsigned DF) const {
   DF *= getDuplicationFactor();
   if (DF <= 1)
     return this;

diff --git a/lib/Transforms/Utils/AddDiscriminators.cpp b/lib/Transforms/Utils/AddDiscriminators.cpp
index eb9f9ed45ffd2b7a0345f86f9f64bb5c0abb1963..ee0973002c47de5f8093cde61923292a623cd80f 100644 (file)
--- a/lib/Transforms/Utils/AddDiscriminators.cpp
+++ b/lib/Transforms/Utils/AddDiscriminators.cpp
@@ -208,7 +208,7 @@ static bool addDiscriminators(Function &F) {
       // Only the lowest 7 bits are used to represent a discriminator to fit
       // it in 1 byte ULEB128 representation.
       unsigned Discriminator = R.second ? ++LDM[L] : LDM[L];
-      auto NewDIL = DIL->setBaseDiscriminator(Discriminator);
+      auto NewDIL = DIL->cloneWithBaseDiscriminator(Discriminator);
       if (!NewDIL) {
         LLVM_DEBUG(dbgs() << "Could not encode discriminator: "
                           << DIL->getFilename() << ":" << DIL->getLine() << ":"
@@ -245,7 +245,7 @@ static bool addDiscriminators(Function &F) {
           std::make_pair(CurrentDIL->getFilename(), CurrentDIL->getLine());
       if (!CallLocations.insert(L).second) {
         unsigned Discriminator = ++LDM[L];
-        auto NewDIL = CurrentDIL->setBaseDiscriminator(Discriminator);
+        auto NewDIL = CurrentDIL->cloneWithBaseDiscriminator(Discriminator);
         if (!NewDIL) {
           LLVM_DEBUG(dbgs()
                      << "Could not encode discriminator: "

diff --git a/lib/Transforms/Utils/LoopUnroll.cpp b/lib/Transforms/Utils/LoopUnroll.cpp
index a2f1a79a1c8b8992cf796bdeecf9e35b0285ddcd..0d62a81c923f52ad17ec2606d8a2abe29f082da4 100644 (file)
--- a/lib/Transforms/Utils/LoopUnroll.cpp
+++ b/lib/Transforms/Utils/LoopUnroll.cpp
@@ -598,7 +598,7 @@ LoopUnrollResult llvm::UnrollLoop(
       for (Instruction &I : *BB)
         if (!isa<DbgInfoIntrinsic>(&I))
           if (const DILocation *DIL = I.getDebugLoc()) {
-            auto NewDIL = DIL->cloneWithDuplicationFactor(Count);
+            auto NewDIL = DIL->cloneByMultiplyingDuplicationFactor(Count);
             if (NewDIL)
               I.setDebugLoc(NewDIL.getValue());
             else

diff --git a/lib/Transforms/Utils/LoopUnrollAndJam.cpp b/lib/Transforms/Utils/LoopUnrollAndJam.cpp
index c3fc636acd6306a7ce7072465d3f7a8946bf02ca..d5ccadde3f92f7f6b06377b4af8bbbd9845329e3 100644 (file)
--- a/lib/Transforms/Utils/LoopUnrollAndJam.cpp
+++ b/lib/Transforms/Utils/LoopUnrollAndJam.cpp
@@ -300,7 +300,7 @@ LoopUnrollResult llvm::UnrollAndJamLoop(
       for (Instruction &I : *BB)
         if (!isa<DbgInfoIntrinsic>(&I))
           if (const DILocation *DIL = I.getDebugLoc()) {
-            auto NewDIL = DIL->cloneWithDuplicationFactor(Count);
+            auto NewDIL = DIL->cloneByMultiplyingDuplicationFactor(Count);
             if (NewDIL)
               I.setDebugLoc(NewDIL.getValue());
             else

diff --git a/lib/Transforms/Vectorize/LoopVectorize.cpp b/lib/Transforms/Vectorize/LoopVectorize.cpp
index 58b86df1e9fa965780ae0a8b69757e75ac2e94c9..da9f01a2f591784596680f7430361a909404eb45 100644 (file)
--- a/lib/Transforms/Vectorize/LoopVectorize.cpp
+++ b/lib/Transforms/Vectorize/LoopVectorize.cpp
@@ -767,7 +767,7 @@ void InnerLoopVectorizer::setDebugLocFromInst(IRBuilder<> &B, const Value *Ptr)
     const DILocation *DIL = Inst->getDebugLoc();
     if (DIL && Inst->getFunction()->isDebugInfoForProfiling() &&
         !isa<DbgInfoIntrinsic>(Inst)) {
-      auto NewDIL = DIL->cloneWithDuplicationFactor(UF * VF);
+      auto NewDIL = DIL->cloneByMultiplyingDuplicationFactor(UF * VF);
       if (NewDIL)
         B.SetCurrentDebugLocation(NewDIL.getValue());
       else

diff --git a/unittests/IR/MetadataTest.cpp b/unittests/IR/MetadataTest.cpp
index 20e2e6e8c983bb3480024ecec36418b435922b28..c66bbde818217468005a9aaf2b56f707f47e5a54 100644 (file)
--- a/unittests/IR/MetadataTest.cpp
+++ b/unittests/IR/MetadataTest.cpp
@@ -1049,35 +1049,41 @@ TEST_F(DILocationTest, discriminatorSpecialCases) {
   EXPECT_EQ(0U, L1->getBaseDiscriminator());
   EXPECT_EQ(1U, L1->getDuplicationFactor());
 
-  auto L2 = L1->setBaseDiscriminator(1).getValue();
+  EXPECT_EQ(L1, L1->cloneWithBaseDiscriminator(0).getValue());
+  EXPECT_EQ(L1, L1->cloneByMultiplyingDuplicationFactor(0).getValue());
+  EXPECT_EQ(L1, L1->cloneByMultiplyingDuplicationFactor(1).getValue());
+
+  auto L2 = L1->cloneWithBaseDiscriminator(1).getValue();
   EXPECT_EQ(0U, L1->getBaseDiscriminator());
   EXPECT_EQ(1U, L1->getDuplicationFactor());
 
   EXPECT_EQ(1U, L2->getBaseDiscriminator());
   EXPECT_EQ(1U, L2->getDuplicationFactor());
 
-  auto L3 = L2->cloneWithDuplicationFactor(2).getValue();
+  auto L3 = L2->cloneByMultiplyingDuplicationFactor(2).getValue();
   EXPECT_EQ(1U, L3->getBaseDiscriminator());
   EXPECT_EQ(2U, L3->getDuplicationFactor());
 
-  auto L4 = L3->cloneWithDuplicationFactor(4).getValue();
+  EXPECT_EQ(L2, L2->cloneByMultiplyingDuplicationFactor(1).getValue());
+
+  auto L4 = L3->cloneByMultiplyingDuplicationFactor(4).getValue();
   EXPECT_EQ(1U, L4->getBaseDiscriminator());
   EXPECT_EQ(8U, L4->getDuplicationFactor());
 
-  auto L5 = L4->setBaseDiscriminator(2).getValue();
+  auto L5 = L4->cloneWithBaseDiscriminator(2).getValue();
   EXPECT_EQ(2U, L5->getBaseDiscriminator());
-  EXPECT_EQ(1U, L5->getDuplicationFactor());
+  EXPECT_EQ(8U, L5->getDuplicationFactor());
 
   // Check extreme cases
-  auto L6 = L1->setBaseDiscriminator(0xfff).getValue();
+  auto L6 = L1->cloneWithBaseDiscriminator(0xfff).getValue();
   EXPECT_EQ(0xfffU, L6->getBaseDiscriminator());
-  EXPECT_EQ(
-      0xfffU,
-      L6->cloneWithDuplicationFactor(0xfff).getValue()->getDuplicationFactor());
+  EXPECT_EQ(0xfffU, L6->cloneByMultiplyingDuplicationFactor(0xfff)
+                        .getValue()
+                        ->getDuplicationFactor());
 
   // Check we return None for unencodable cases.
-  EXPECT_EQ(None, L4->setBaseDiscriminator(0x1000));
-  EXPECT_EQ(None, L4->cloneWithDuplicationFactor(0x1000));
+  EXPECT_EQ(None, L4->cloneWithBaseDiscriminator(0x1000));
+  EXPECT_EQ(None, L4->cloneByMultiplyingDuplicationFactor(0x1000));
 }