Skip to content

Commit d90780e

Browse files
committed
param-comments and simplify lambda with early exits
1 parent 5ec19d6 commit d90780e

File tree

2 files changed

+90
-66
lines changed

2 files changed

+90
-66
lines changed

llvm/lib/IR/DebugInfoMetadata.cpp

Lines changed: 36 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -351,40 +351,43 @@ DILocation *DILocation::getMergedLocation(DILocation *LocA, DILocation *LocB) {
351351
unsigned Line = SameLine ? L1->getLine() : 0;
352352
unsigned Col = SameLine && SameCol ? L1->getColumn() : 0;
353353
bool IsImplicitCode = L1->isImplicitCode() && L2->isImplicitCode();
354+
355+
// Discard source location atom if the line becomes 0. And there's nothing
356+
// further to do if neither location has an atom number.
357+
if (!SameLine || !(L1->getAtomGroup() || L2->getAtomGroup()))
358+
return DILocation::get(C, Line, Col, Scope, InlinedAt, IsImplicitCode,
359+
/*AtomGroup*/ 0, /*AtomRank*/ 0);
360+
354361
uint64_t Group = 0;
355362
uint64_t Rank = 0;
356-
if (SameLine) {
357-
if (L1->getAtomGroup() || L2->getAtomGroup()) {
358-
// If we're preserving the same matching inlined-at field we can
359-
// preserve the atom.
360-
if (LocBIA == LocAIA && InlinedAt == LocBIA) {
361-
// Deterministically keep the lowest non-zero ranking atom group
362-
// number.
363-
// FIXME: It would be nice if we could track that an instruction
364-
// belongs to two source atoms.
365-
bool UseL1Atom = [L1, L2]() {
366-
if (L1->getAtomRank() == L2->getAtomRank()) {
367-
// Arbitrarily choose the lowest non-zero group number.
368-
if (!L1->getAtomGroup() || !L2->getAtomGroup())
369-
return !L2->getAtomGroup();
370-
return L1->getAtomGroup() < L2->getAtomGroup();
371-
}
372-
// Choose the lowest non-zero rank.
373-
if (!L1->getAtomRank() || !L2->getAtomRank())
374-
return !L2->getAtomRank();
375-
return L1->getAtomRank() < L2->getAtomRank();
376-
}();
377-
Group = UseL1Atom ? L1->getAtomGroup() : L2->getAtomGroup();
378-
Rank = UseL1Atom ? L1->getAtomRank() : L2->getAtomRank();
379-
} else {
380-
// If either instruction is part of a source atom, reassign it a new
381-
// atom group. This essentially regresses to non-key-instructions
382-
// behaviour (now that it's the only instruction in its group it'll
383-
// probably get is_stmt applied).
384-
Group = C.incNextAtomGroup();
385-
Rank = 1;
363+
// If we're preserving the same matching inlined-at field we can
364+
// preserve the atom.
365+
if (LocBIA == LocAIA && InlinedAt == LocBIA) {
366+
// Deterministically keep the lowest non-zero ranking atom group
367+
// number.
368+
// FIXME: It would be nice if we could track that an instruction
369+
// belongs to two source atoms.
370+
bool UseL1Atom = [L1, L2]() {
371+
if (L1->getAtomRank() == L2->getAtomRank()) {
372+
// Arbitrarily choose the lowest non-zero group number.
373+
if (!L1->getAtomGroup() || !L2->getAtomGroup())
374+
return !L2->getAtomGroup();
375+
return L1->getAtomGroup() < L2->getAtomGroup();
386376
}
387-
}
377+
// Choose the lowest non-zero rank.
378+
if (!L1->getAtomRank() || !L2->getAtomRank())
379+
return !L2->getAtomRank();
380+
return L1->getAtomRank() < L2->getAtomRank();
381+
}();
382+
Group = UseL1Atom ? L1->getAtomGroup() : L2->getAtomGroup();
383+
Rank = UseL1Atom ? L1->getAtomRank() : L2->getAtomRank();
384+
} else {
385+
// If either instruction is part of a source atom, reassign it a new
386+
// atom group. This essentially regresses to non-key-instructions
387+
// behaviour (now that it's the only instruction in its group it'll
388+
// probably get is_stmt applied).
389+
Group = C.incNextDILocationAtomGroup();
390+
Rank = 1;
388391
}
389392
return DILocation::get(C, Line, Col, Scope, InlinedAt, IsImplicitCode,
390393
Group, Rank);
@@ -416,7 +419,8 @@ DILocation *DILocation::getMergedLocation(DILocation *LocA, DILocation *LocB) {
416419
// way to handle this.
417420
// Key Instructions: it's fine to drop atom group and rank here, as line 0
418421
// is a nonsensical is_stmt location.
419-
return DILocation::get(C, 0, 0, LocA->getScope(), nullptr, false, 0, 0);
422+
return DILocation::get(C, 0, 0, LocA->getScope(), nullptr, false,
423+
/*AtomGroup*/ 0, /*AtomRank*/ 0);
420424
}
421425

422426
std::optional<unsigned>

llvm/unittests/IR/MetadataTest.cpp

Lines changed: 54 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1482,68 +1482,83 @@ TEST_F(DILocationTest, Merge) {
14821482
#endif
14831483
// Identical, including source atom numbers.
14841484
{
1485-
auto *A = DILocation::get(Context, 2, 7, N, nullptr, false, 1, 1);
1486-
auto *B = DILocation::get(Context, 2, 7, N, nullptr, false, 1, 1);
1485+
auto *A = DILocation::get(Context, 2, 7, N, nullptr, false, /*AtomGroup*/ 1,
1486+
/*AtomRank*/ 1);
1487+
auto *B = DILocation::get(Context, 2, 7, N, nullptr, false, /*AtomGroup*/ 1,
1488+
/*AtomRank*/ 1);
14871489
auto *M = DILocation::getMergedLocation(A, B);
1488-
EXPECT_ATOM(M, 1u, 1u);
1490+
EXPECT_ATOM(M, /*AtomGroup*/ 1u, 1u);
14891491
// DILocations are uniqued, so we can check equality by ptr.
14901492
EXPECT_EQ(M, DILocation::getMergedLocation(A, B));
14911493
}
14921494

14931495
// Identical but different atom ranks (same atom) - choose the lowest nonzero
14941496
// rank.
14951497
{
1496-
auto *A = DILocation::get(Context, 2, 7, N, nullptr, false, 1, 1);
1497-
auto *B = DILocation::get(Context, 2, 7, N, nullptr, false, 1, 2);
1498+
auto *A = DILocation::get(Context, 2, 7, N, nullptr, false, /*AtomGroup*/ 1,
1499+
/*AtomRank*/ 1);
1500+
auto *B = DILocation::get(Context, 2, 7, N, nullptr, false, /*AtomGroup*/ 1,
1501+
/*AtomRank*/ 2);
14981502
auto *M = DILocation::getMergedLocation(A, B);
1499-
EXPECT_ATOM(M, 1u, 1u);
1503+
EXPECT_ATOM(M, /*AtomGroup*/ 1u, /*AtomRank*/ 1u);
15001504
EXPECT_EQ(M, DILocation::getMergedLocation(B, A));
15011505

1502-
A = DILocation::get(Context, 2, 7, N, nullptr, false, 1, 0);
1503-
B = DILocation::get(Context, 2, 7, N, nullptr, false, 1, 2);
1506+
A = DILocation::get(Context, 2, 7, N, nullptr, false, /*AtomGroup*/ 1,
1507+
/*AtomRank*/ 0);
1508+
B = DILocation::get(Context, 2, 7, N, nullptr, false, /*AtomGroup*/ 1,
1509+
/*AtomRank*/ 2);
15041510
M = DILocation::getMergedLocation(A, B);
1505-
EXPECT_ATOM(M, 1u, 2u);
1511+
EXPECT_ATOM(M, /*AtomGroup*/ 1u, /*AtomRank*/ 2u);
15061512
EXPECT_EQ(M, DILocation::getMergedLocation(B, A));
15071513
}
15081514

15091515
// Identical but different atom ranks (different atom) - choose the lowest
15101516
// nonzero rank.
15111517
{
1512-
auto *A = DILocation::get(Context, 2, 7, N, nullptr, false, 1, 1);
1513-
auto *B = DILocation::get(Context, 2, 7, N, nullptr, false, 2, 2);
1518+
auto *A = DILocation::get(Context, 2, 7, N, nullptr, false, /*AtomGroup*/ 1,
1519+
/*AtomRank*/ 1);
1520+
auto *B = DILocation::get(Context, 2, 7, N, nullptr, false, /*AtomGroup*/ 2,
1521+
/*AtomRank*/ 2);
15141522
auto *M = DILocation::getMergedLocation(A, B);
15151523
EXPECT_ATOM(M, 1u, 1u);
15161524
EXPECT_EQ(M, DILocation::getMergedLocation(B, A));
15171525

1518-
A = DILocation::get(Context, 2, 7, N, nullptr, false, 1, 0);
1519-
B = DILocation::get(Context, 2, 7, N, nullptr, false, 2, 2);
1526+
A = DILocation::get(Context, 2, 7, N, nullptr, false, /*AtomGroup*/ 1,
1527+
/*AtomRank*/ 0);
1528+
B = DILocation::get(Context, 2, 7, N, nullptr, false, /*AtomGroup*/ 2,
1529+
/*AtomRank*/ 2);
15201530
M = DILocation::getMergedLocation(A, B);
1521-
EXPECT_ATOM(M, 2u, 2u);
1531+
EXPECT_ATOM(M, /*AtomGroup*/ 2u, /*AtomRank*/ 2u);
15221532
EXPECT_EQ(M, DILocation::getMergedLocation(B, A));
15231533
}
15241534

15251535
// Identical but equal atom rank (different atom) - choose the lowest non-zero
15261536
// group (arbitrary choice for deterministic behaviour).
15271537
{
1528-
auto *A = DILocation::get(Context, 2, 7, N, nullptr, false, 1, 1);
1529-
auto *B = DILocation::get(Context, 2, 7, N, nullptr, false, 2, 1);
1538+
auto *A = DILocation::get(Context, 2, 7, N, nullptr, false, /*AtomGroup*/ 1,
1539+
/*AtomRank*/ 1);
1540+
auto *B = DILocation::get(Context, 2, 7, N, nullptr, false, /*AtomGroup*/ 2,
1541+
/*AtomRank*/ 1);
15301542
auto *M = DILocation::getMergedLocation(A, B);
15311543
EXPECT_ATOM(M, 1u, 1u);
15321544
EXPECT_EQ(M, DILocation::getMergedLocation(B, A));
15331545

1534-
A = DILocation::get(Context, 2, 7, N, nullptr, false, 0, 1);
1535-
B = DILocation::get(Context, 2, 7, N, nullptr, false, 2, 1);
1546+
A = DILocation::get(Context, 2, 7, N, nullptr, false, /*AtomGroup*/ 0,
1547+
/*AtomRank*/ 1);
1548+
B = DILocation::get(Context, 2, 7, N, nullptr, false, /*AtomGroup*/ 2,
1549+
/*AtomRank*/ 1);
15361550
M = DILocation::getMergedLocation(A, B);
1537-
EXPECT_ATOM(M, 2u, 1u);
1551+
EXPECT_ATOM(M, /*AtomGroup*/ 2u, /*AtomRank*/ 1u);
15381552
EXPECT_EQ(M, DILocation::getMergedLocation(B, A));
15391553
}
15401554

15411555
// Completely different except same atom numbers. Zero out the atoms.
15421556
{
15431557
auto *I = DILocation::get(Context, 2, 7, N);
1544-
auto *A = DILocation::get(Context, 1, 6, S, I, false, 1, 1);
1545-
auto *B =
1546-
DILocation::get(Context, 2, 7, getSubprogram(), nullptr, false, 1, 1);
1558+
auto *A = DILocation::get(Context, 1, 6, S, I, false, /*AtomGroup*/ 1,
1559+
/*AtomRank*/ 1);
1560+
auto *B = DILocation::get(Context, 2, 7, getSubprogram(), nullptr, false,
1561+
/*AtomGroup*/ 1, /*AtomRank*/ 1);
15471562
auto *M = DILocation::getMergedLocation(A, B);
15481563
EXPECT_EQ(0u, M->getLine());
15491564
EXPECT_EQ(0u, M->getColumn());
@@ -1557,16 +1572,20 @@ TEST_F(DILocationTest, Merge) {
15571572
{
15581573
auto *I = DILocation::get(Context, 1, 7, N);
15591574
auto *F = getSubprogram();
1560-
auto *A = DILocation::get(Context, 1, 1, F, I, false, 1, 2);
1561-
auto *B = DILocation::get(Context, 1, 1, F, I, false, 2, 1);
1575+
auto *A = DILocation::get(Context, 1, 1, F, I, false, /*AtomGroup*/ 1,
1576+
/*AtomRank*/ 2);
1577+
auto *B = DILocation::get(Context, 1, 1, F, I, false, /*AtomGroup*/ 2,
1578+
/*AtomRank*/ 1);
15621579
auto *M = DILocation::getMergedLocation(A, B);
1563-
EXPECT_ATOM(M, 2u, 1u);
1580+
EXPECT_ATOM(M, 2u, /*AtomRank*/ 1u);
15641581
EXPECT_EQ(M, DILocation::getMergedLocation(B, A));
15651582

1566-
A = DILocation::get(Context, 1, 1, F, I, false, 1, 2);
1567-
B = DILocation::get(Context, 1, 1, F, I, false, 2, 0);
1583+
A = DILocation::get(Context, 1, 1, F, I, false, /*AtomGroup*/ 1,
1584+
/*AtomRank*/ 2);
1585+
B = DILocation::get(Context, 1, 1, F, I, false, /*AtomGroup*/ 2,
1586+
/*AtomRank*/ 0);
15681587
M = DILocation::getMergedLocation(A, B);
1569-
EXPECT_ATOM(M, 1u, 2u);
1588+
EXPECT_ATOM(M, /*AtomGroup*/ 1u, /*AtomRank*/ 2u);
15701589
EXPECT_EQ(M, DILocation::getMergedLocation(B, A));
15711590
}
15721591

@@ -1582,24 +1601,25 @@ TEST_F(DILocationTest, Merge) {
15821601
auto *FY = getSubprogram();
15831602
auto *FZ = getSubprogram();
15841603
auto *Z4 = DILocation::get(Context, 1, 4, FZ);
1585-
auto *Y3IntoZ4 = DILocation::get(Context, 1, 3, FY, Z4, false, 1, 1);
1604+
auto *Y3IntoZ4 = DILocation::get(Context, 1, 3, FY, Z4, false,
1605+
/*AtomGroup*/ 1, /*AtomRank*/ 1);
15861606
auto *Y2IntoZ4 = DILocation::get(Context, 1, 2, FY, Z4);
15871607
auto *X1IntoY2 = DILocation::get(Context, 1, 1, FX, Y2IntoZ4);
15881608
auto *M = DILocation::getMergedLocation(X1IntoY2, Y3IntoZ4);
15891609
EXPECT_EQ(M->getScope(), FY);
15901610
EXPECT_EQ(M->getInlinedAt()->getScope(), FZ);
1591-
EXPECT_ATOM(M, 2u, 1u);
1611+
EXPECT_ATOM(M, /*AtomGroup*/ 2u, /*AtomRank*/ 1u);
15921612

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

15961616
// Same again, even if the atom numbers match.
1597-
auto *X1IntoY2SameAtom =
1598-
DILocation::get(Context, 1, 1, FX, Y2IntoZ4, false, 1, 1);
1617+
auto *X1IntoY2SameAtom = DILocation::get(Context, 1, 1, FX, Y2IntoZ4, false,
1618+
/*AtomGroup*/ 1, /*AtomRank*/ 1);
15991619
M = DILocation::getMergedLocation(X1IntoY2SameAtom, Y3IntoZ4);
1600-
EXPECT_ATOM(M, 4u, 1u);
1620+
EXPECT_ATOM(M, /*AtomGroup*/ 4u, /*AtomRank*/ 1u);
16011621
M = DILocation::getMergedLocation(Y3IntoZ4, X1IntoY2SameAtom);
1602-
EXPECT_ATOM(M, 5u, 1u);
1622+
EXPECT_ATOM(M, /*AtomGroup*/ 5u, /*AtomRank*/ 1u);
16031623
}
16041624
#undef EXPECT_ATOM
16051625
}

0 commit comments

Comments
 (0)