From: Mircea Trofin Date: Thu, 24 Jan 2019 00:10:25 +0000 (+0000) Subject: [llvm] Clarify responsiblity of some of DILocation discriminator APIs X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=06831ab511b060e3c59736ca8a1ad73c0c2ed610;p=llvm [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 849dc13ba96..40e6873eba7 100644 --- 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 setBaseDiscriminator(unsigned BD) const; + inline Optional 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 cloneWithDuplicationFactor(unsigned DF) const; + inline Optional 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 DILocation::setBaseDiscriminator(unsigned D) const { - if (D == 0) +Optional 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 Encoded = encodeDiscriminator(D, DF, CI)) + return cloneWithDiscriminator(*Encoded); + return None; } -Optional DILocation::cloneWithDuplicationFactor(unsigned DF) const { +Optional 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 eb9f9ed45ff..ee0973002c4 100644 --- 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 a2f1a79a1c8..0d62a81c923 100644 --- 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(&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 c3fc636acd6..d5ccadde3f9 100644 --- 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(&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 58b86df1e9f..da9f01a2f59 100644 --- 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(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 20e2e6e8c98..c66bbde8182 100644 --- 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)); }