Skip to content

[KeyInstr] Merge atoms in DILocation::getMergedLocation #133480

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 6, 2025
Merged

[KeyInstr] Merge atoms in DILocation::getMergedLocation #133480

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5ec19d6
[KeyInstr] Merge atoms in DILocation::getMergedLocation
OCHyams Mar 21, 2025
d90780e
param-comments and simplify lambda with early exits
OCHyams May 1, 2025
5feaa9c
use new function names
OCHyams May 1, 2025
6c4caf3
fix unittest
OCHyams May 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 52 additions & 6 deletions llvm/lib/IR/DebugInfoMetadata.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -306,11 +306,15 @@ DILocation *DILocation::getMergedLocation(DILocation *LocA, DILocation *LocB) {

// Merge the two locations if possible, using the supplied
// inlined-at location for the created location.
auto MergeLocPair = [&C](const DILocation *L1, const DILocation *L2,
DILocation *InlinedAt) -> DILocation * {
auto *LocAIA = LocA->getInlinedAt();
auto *LocBIA = LocB->getInlinedAt();
auto MergeLocPair = [&C, LocAIA,
LocBIA](const DILocation *L1, const DILocation *L2,
DILocation *InlinedAt) -> DILocation * {
if (L1 == L2)
return DILocation::get(C, L1->getLine(), L1->getColumn(), L1->getScope(),
InlinedAt);
InlinedAt, L1->isImplicitCode(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically copying L1->isImplicitCode() here is a change in behaviour - normally that flag would be effectively dropped here.

L1->getAtomGroup(), L1->getAtomRank());

// If the locations originate from different subprograms we can't produce
// a common location.
Expand Down Expand Up @@ -346,8 +350,47 @@ DILocation *DILocation::getMergedLocation(DILocation *LocA, DILocation *LocB) {
bool SameCol = L1->getColumn() == L2->getColumn();
unsigned Line = SameLine ? L1->getLine() : 0;
unsigned Col = SameLine && SameCol ? L1->getColumn() : 0;

return DILocation::get(C, Line, Col, Scope, InlinedAt);
bool IsImplicitCode = L1->isImplicitCode() && L2->isImplicitCode();

// Discard source location atom if the line becomes 0. And there's nothing
// further to do if neither location has an atom number.
if (!SameLine || !(L1->getAtomGroup() || L2->getAtomGroup()))
return DILocation::get(C, Line, Col, Scope, InlinedAt, IsImplicitCode,
/*AtomGroup*/ 0, /*AtomRank*/ 0);

uint64_t Group = 0;
uint64_t Rank = 0;
// If we're preserving the same matching inlined-at field we can
// preserve the atom.
if (LocBIA == LocAIA && InlinedAt == LocBIA) {
// Deterministically keep the lowest non-zero ranking atom group
// number.
// FIXME: It would be nice if we could track that an instruction
// belongs to two source atoms.
bool UseL1Atom = [L1, L2]() {
if (L1->getAtomRank() == L2->getAtomRank()) {
// Arbitrarily choose the lowest non-zero group number.
if (!L1->getAtomGroup() || !L2->getAtomGroup())
return !L2->getAtomGroup();
return L1->getAtomGroup() < L2->getAtomGroup();
}
// Choose the lowest non-zero rank.
if (!L1->getAtomRank() || !L2->getAtomRank())
return !L2->getAtomRank();
return L1->getAtomRank() < L2->getAtomRank();
}();
Group = UseL1Atom ? L1->getAtomGroup() : L2->getAtomGroup();
Rank = UseL1Atom ? L1->getAtomRank() : L2->getAtomRank();
} else {
// If either instruction is part of a source atom, reassign it a new
// atom group. This essentially regresses to non-key-instructions
// behaviour (now that it's the only instruction in its group it'll
// probably get is_stmt applied).
Group = C.incNextDILocationAtomGroup();
Rank = 1;
}
return DILocation::get(C, Line, Col, Scope, InlinedAt, IsImplicitCode,
Group, Rank);
};

DILocation *Result = ARIt != ALocs.rend() ? (*ARIt)->getInlinedAt() : nullptr;
Expand All @@ -374,7 +417,10 @@ DILocation *DILocation::getMergedLocation(DILocation *LocA, DILocation *LocB) {
// historically picked A's scope, and a nullptr inlined-at location, so that
// behavior is mimicked here but I am not sure if this is always the correct
// way to handle this.
return DILocation::get(C, 0, 0, LocA->getScope(), nullptr);
// Key Instructions: it's fine to drop atom group and rank here, as line 0
// is a nonsensical is_stmt location.
return DILocation::get(C, 0, 0, LocA->getScope(), nullptr, false,
/*AtomGroup*/ 0, /*AtomRank*/ 0);
}

std::optional<unsigned>
Expand Down
154 changes: 154 additions & 0 deletions llvm/unittests/IR/MetadataTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1468,6 +1468,160 @@ TEST_F(DILocationTest, Merge) {
EXPECT_EQ(N, M2->getScope());
PickMergedSourceLocations = false;
}

#ifdef EXPERIMENTAL_KEY_INSTRUCTIONS
#define EXPECT_ATOM(Loc, Group, Rank) \
EXPECT_EQ(Group, M->getAtomGroup()); \
EXPECT_EQ(Rank, M->getAtomRank());
#else
#define EXPECT_ATOM(Loc, Group, Rank) \
EXPECT_EQ(0u, M->getAtomGroup()); \
EXPECT_EQ(0u, M->getAtomRank()); \
(void)Group; \
(void)Rank;
#endif
// Identical, including source atom numbers.
{
auto *A = DILocation::get(Context, 2, 7, N, nullptr, false, /*AtomGroup*/ 1,
/*AtomRank*/ 1);
auto *B = DILocation::get(Context, 2, 7, N, nullptr, false, /*AtomGroup*/ 1,
/*AtomRank*/ 1);
auto *M = DILocation::getMergedLocation(A, B);
EXPECT_ATOM(M, /*AtomGroup*/ 1u, 1u);
// DILocations are uniqued, so we can check equality by ptr.
EXPECT_EQ(M, DILocation::getMergedLocation(A, B));
}

// Identical but different atom ranks (same atom) - choose the lowest nonzero
// rank.
{
auto *A = DILocation::get(Context, 2, 7, N, nullptr, false, /*AtomGroup*/ 1,
/*AtomRank*/ 1);
auto *B = DILocation::get(Context, 2, 7, N, nullptr, false, /*AtomGroup*/ 1,
/*AtomRank*/ 2);
auto *M = DILocation::getMergedLocation(A, B);
EXPECT_ATOM(M, /*AtomGroup*/ 1u, /*AtomRank*/ 1u);
EXPECT_EQ(M, DILocation::getMergedLocation(B, A));

A = DILocation::get(Context, 2, 7, N, nullptr, false, /*AtomGroup*/ 1,
/*AtomRank*/ 0);
B = DILocation::get(Context, 2, 7, N, nullptr, false, /*AtomGroup*/ 1,
/*AtomRank*/ 2);
M = DILocation::getMergedLocation(A, B);
EXPECT_ATOM(M, /*AtomGroup*/ 1u, /*AtomRank*/ 2u);
EXPECT_EQ(M, DILocation::getMergedLocation(B, A));
}

// Identical but different atom ranks (different atom) - choose the lowest
// nonzero rank.
{
auto *A = DILocation::get(Context, 2, 7, N, nullptr, false, /*AtomGroup*/ 1,
/*AtomRank*/ 1);
auto *B = DILocation::get(Context, 2, 7, N, nullptr, false, /*AtomGroup*/ 2,
/*AtomRank*/ 2);
auto *M = DILocation::getMergedLocation(A, B);
EXPECT_ATOM(M, 1u, 1u);
EXPECT_EQ(M, DILocation::getMergedLocation(B, A));

A = DILocation::get(Context, 2, 7, N, nullptr, false, /*AtomGroup*/ 1,
/*AtomRank*/ 0);
B = DILocation::get(Context, 2, 7, N, nullptr, false, /*AtomGroup*/ 2,
/*AtomRank*/ 2);
M = DILocation::getMergedLocation(A, B);
EXPECT_ATOM(M, /*AtomGroup*/ 2u, /*AtomRank*/ 2u);
EXPECT_EQ(M, DILocation::getMergedLocation(B, A));
}

// Identical but equal atom rank (different atom) - choose the lowest non-zero
// group (arbitrary choice for deterministic behaviour).
{
auto *A = DILocation::get(Context, 2, 7, N, nullptr, false, /*AtomGroup*/ 1,
/*AtomRank*/ 1);
auto *B = DILocation::get(Context, 2, 7, N, nullptr, false, /*AtomGroup*/ 2,
/*AtomRank*/ 1);
auto *M = DILocation::getMergedLocation(A, B);
EXPECT_ATOM(M, 1u, 1u);
EXPECT_EQ(M, DILocation::getMergedLocation(B, A));

A = DILocation::get(Context, 2, 7, N, nullptr, false, /*AtomGroup*/ 0,
/*AtomRank*/ 1);
B = DILocation::get(Context, 2, 7, N, nullptr, false, /*AtomGroup*/ 2,
/*AtomRank*/ 1);
M = DILocation::getMergedLocation(A, B);
EXPECT_ATOM(M, /*AtomGroup*/ 2u, /*AtomRank*/ 1u);
EXPECT_EQ(M, DILocation::getMergedLocation(B, A));
}

// Completely different except same atom numbers. Zero out the atoms.
{
auto *I = DILocation::get(Context, 2, 7, N);
auto *A = DILocation::get(Context, 1, 6, S, I, false, /*AtomGroup*/ 1,
/*AtomRank*/ 1);
auto *B = DILocation::get(Context, 2, 7, getSubprogram(), nullptr, false,
/*AtomGroup*/ 1, /*AtomRank*/ 1);
auto *M = DILocation::getMergedLocation(A, B);
EXPECT_EQ(0u, M->getLine());
EXPECT_EQ(0u, M->getColumn());
EXPECT_TRUE(isa<DILocalScope>(M->getScope()));
EXPECT_EQ(S, M->getScope());
EXPECT_EQ(nullptr, M->getInlinedAt());
}

// Same inlined-at chain but different atoms. Choose the lowest
// non-zero group (arbitrary choice for deterministic behaviour).
{
auto *I = DILocation::get(Context, 1, 7, N);
auto *F = getSubprogram();
auto *A = DILocation::get(Context, 1, 1, F, I, false, /*AtomGroup*/ 1,
/*AtomRank*/ 2);
auto *B = DILocation::get(Context, 1, 1, F, I, false, /*AtomGroup*/ 2,
/*AtomRank*/ 2);
auto *M = DILocation::getMergedLocation(A, B);
EXPECT_ATOM(M, /*AtomGroup*/ 1u, /*AtomRank*/ 2u);
EXPECT_EQ(M, DILocation::getMergedLocation(B, A));

A = DILocation::get(Context, 1, 1, F, I, false, /*AtomGroup*/ 1,
/*AtomRank*/ 2);
B = DILocation::get(Context, 1, 1, F, I, false, /*AtomGroup*/ 2,
/*AtomRank*/ 0);
M = DILocation::getMergedLocation(A, B);
EXPECT_ATOM(M, /*AtomGroup*/ 1u, /*AtomRank*/ 2u);
EXPECT_EQ(M, DILocation::getMergedLocation(B, A));
}

// Partially equal inlined-at chain but different atoms. Generate a new atom
// group (if either have a group number). This configuration seems unlikely
// to occur as line numbers must match, but isn't impossible.
{
Comment on lines +1592 to +1595
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe it's a desired and supported configuration for this to occur when a function is inlined multiple times then the copies are merged: we keep the line number of the instruction, but put line-zero in the inlinedAt field so that the inlining-callstack refers to a nonexistant location. Thus, we're saying "It's definitely this line, but we can't tell you what inlining context it came from, there were a few".

I don't know whether it makes sense to try and support key instructions in that situation; I feel that falling back to old stepping behaviour is fine in these rare circumstances.

// Reset global counter to ensure EXPECT numbers line up.
Context.pImpl->NextAtomGroup = 1;
// x1 -> y2 -> z4
// y3 -> z4
auto *FX = getSubprogram();
auto *FY = getSubprogram();
auto *FZ = getSubprogram();
auto *Z4 = DILocation::get(Context, 1, 4, FZ);
auto *Y3IntoZ4 = DILocation::get(Context, 1, 3, FY, Z4, false,
/*AtomGroup*/ 1, /*AtomRank*/ 1);
auto *Y2IntoZ4 = DILocation::get(Context, 1, 2, FY, Z4);
auto *X1IntoY2 = DILocation::get(Context, 1, 1, FX, Y2IntoZ4);
auto *M = DILocation::getMergedLocation(X1IntoY2, Y3IntoZ4);
EXPECT_EQ(M->getScope(), FY);
EXPECT_EQ(M->getInlinedAt()->getScope(), FZ);
EXPECT_ATOM(M, /*AtomGroup*/ 2u, /*AtomRank*/ 1u);

// This swapped merge will produce a new atom group too.
M = DILocation::getMergedLocation(Y3IntoZ4, X1IntoY2);

// Same again, even if the atom numbers match.
auto *X1IntoY2SameAtom = DILocation::get(Context, 1, 1, FX, Y2IntoZ4, false,
/*AtomGroup*/ 1, /*AtomRank*/ 1);
M = DILocation::getMergedLocation(X1IntoY2SameAtom, Y3IntoZ4);
EXPECT_ATOM(M, /*AtomGroup*/ 4u, /*AtomRank*/ 1u);
M = DILocation::getMergedLocation(Y3IntoZ4, X1IntoY2SameAtom);
EXPECT_ATOM(M, /*AtomGroup*/ 5u, /*AtomRank*/ 1u);
}
#undef EXPECT_ATOM
}

TEST_F(DILocationTest, getDistinct) {
Expand Down