Skip to content

Commit 9a42448

Browse files
OCHyamsOCHyams
committed
param-comments and simplify lambda with early exits
1 parent 2c538d7 commit 9a42448

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
@@ -347,40 +347,43 @@ DILocation *DILocation::getMergedLocation(DILocation *LocA, DILocation *LocB) {
347347
unsigned Line = SameLine ? L1->getLine() : 0;
348348
unsigned Col = SameLine && SameCol ? L1->getColumn() : 0;
349349
bool IsImplicitCode = L1->isImplicitCode() && L2->isImplicitCode();
350+
351+
// Discard source location atom if the line becomes 0. And there's nothing
352+
// further to do if neither location has an atom number.
353+
if (!SameLine || !(L1->getAtomGroup() || L2->getAtomGroup()))
354+
return DILocation::get(C, Line, Col, Scope, InlinedAt, IsImplicitCode,
355+
/*AtomGroup*/ 0, /*AtomRank*/ 0);
356+
350357
uint64_t Group = 0;
351358
uint64_t Rank = 0;
352-
if (SameLine) {
353-
if (L1->getAtomGroup() || L2->getAtomGroup()) {
354-
// If we're preserving the same matching inlined-at field we can
355-
// preserve the atom.
356-
if (LocBIA == LocAIA && InlinedAt == LocBIA) {
357-
// Deterministically keep the lowest non-zero ranking atom group
358-
// number.
359-
// FIXME: It would be nice if we could track that an instruction
360-
// belongs to two source atoms.
361-
bool UseL1Atom = [L1, L2]() {
362-
if (L1->getAtomRank() == L2->getAtomRank()) {
363-
// Arbitrarily choose the lowest non-zero group number.
364-
if (!L1->getAtomGroup() || !L2->getAtomGroup())
365-
return !L2->getAtomGroup();
366-
return L1->getAtomGroup() < L2->getAtomGroup();
367-
}
368-
// Choose the lowest non-zero rank.
369-
if (!L1->getAtomRank() || !L2->getAtomRank())
370-
return !L2->getAtomRank();
371-
return L1->getAtomRank() < L2->getAtomRank();
372-
}();
373-
Group = UseL1Atom ? L1->getAtomGroup() : L2->getAtomGroup();
374-
Rank = UseL1Atom ? L1->getAtomRank() : L2->getAtomRank();
375-
} else {
376-
// If either instruction is part of a source atom, reassign it a new
377-
// atom group. This essentially regresses to non-key-instructions
378-
// behaviour (now that it's the only instruction in its group it'll
379-
// probably get is_stmt applied).
380-
Group = C.incNextAtomGroup();
381-
Rank = 1;
359+
// If we're preserving the same matching inlined-at field we can
360+
// preserve the atom.
361+
if (LocBIA == LocAIA && InlinedAt == LocBIA) {
362+
// Deterministically keep the lowest non-zero ranking atom group
363+
// number.
364+
// FIXME: It would be nice if we could track that an instruction
365+
// belongs to two source atoms.
366+
bool UseL1Atom = [L1, L2]() {
367+
if (L1->getAtomRank() == L2->getAtomRank()) {
368+
// Arbitrarily choose the lowest non-zero group number.
369+
if (!L1->getAtomGroup() || !L2->getAtomGroup())
370+
return !L2->getAtomGroup();
371+
return L1->getAtomGroup() < L2->getAtomGroup();
382372
}
383-
}
373+
// Choose the lowest non-zero rank.
374+
if (!L1->getAtomRank() || !L2->getAtomRank())
375+
return !L2->getAtomRank();
376+
return L1->getAtomRank() < L2->getAtomRank();
377+
}();
378+
Group = UseL1Atom ? L1->getAtomGroup() : L2->getAtomGroup();
379+
Rank = UseL1Atom ? L1->getAtomRank() : L2->getAtomRank();
380+
} else {
381+
// If either instruction is part of a source atom, reassign it a new
382+
// atom group. This essentially regresses to non-key-instructions
383+
// behaviour (now that it's the only instruction in its group it'll
384+
// probably get is_stmt applied).
385+
Group = C.incNextDILocationAtomGroup();
386+
Rank = 1;
384387
}
385388
return DILocation::get(C, Line, Col, Scope, InlinedAt, IsImplicitCode,
386389
Group, Rank);
@@ -412,7 +415,8 @@ DILocation *DILocation::getMergedLocation(DILocation *LocA, DILocation *LocB) {
412415
// way to handle this.
413416
// Key Instructions: it's fine to drop atom group and rank here, as line 0
414417
// is a nonsensical is_stmt location.
415-
return DILocation::get(C, 0, 0, LocA->getScope(), nullptr, false, 0, 0);
418+
return DILocation::get(C, 0, 0, LocA->getScope(), nullptr, false,
419+
/*AtomGroup*/ 0, /*AtomRank*/ 0);
416420
}
417421

418422
std::optional<unsigned>

‎llvm/unittests/IR/MetadataTest.cpp

Lines changed: 54 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1459,68 +1459,83 @@ TEST_F(DILocationTest, Merge) {
14591459
#endif
14601460
// Identical, including source atom numbers.
14611461
{
1462-
auto *A = DILocation::get(Context, 2, 7, N, nullptr, false, 1, 1);
1463-
auto *B = DILocation::get(Context, 2, 7, N, nullptr, false, 1, 1);
1462+
auto *A = DILocation::get(Context, 2, 7, N, nullptr, false, /*AtomGroup*/ 1,
1463+
/*AtomRank*/ 1);
1464+
auto *B = DILocation::get(Context, 2, 7, N, nullptr, false, /*AtomGroup*/ 1,
1465+
/*AtomRank*/ 1);
14641466
auto *M = DILocation::getMergedLocation(A, B);
1465-
EXPECT_ATOM(M, 1u, 1u);
1467+
EXPECT_ATOM(M, /*AtomGroup*/ 1u, 1u);
14661468
// DILocations are uniqued, so we can check equality by ptr.
14671469
EXPECT_EQ(M, DILocation::getMergedLocation(A, B));
14681470
}
14691471

14701472
// Identical but different atom ranks (same atom) - choose the lowest nonzero
14711473
// rank.
14721474
{
1473-
auto *A = DILocation::get(Context, 2, 7, N, nullptr, false, 1, 1);
1474-
auto *B = DILocation::get(Context, 2, 7, N, nullptr, false, 1, 2);
1475+
auto *A = DILocation::get(Context, 2, 7, N, nullptr, false, /*AtomGroup*/ 1,
1476+
/*AtomRank*/ 1);
1477+
auto *B = DILocation::get(Context, 2, 7, N, nullptr, false, /*AtomGroup*/ 1,
1478+
/*AtomRank*/ 2);
14751479
auto *M = DILocation::getMergedLocation(A, B);
1476-
EXPECT_ATOM(M, 1u, 1u);
1480+
EXPECT_ATOM(M, /*AtomGroup*/ 1u, /*AtomRank*/ 1u);
14771481
EXPECT_EQ(M, DILocation::getMergedLocation(B, A));
14781482

1479-
A = DILocation::get(Context, 2, 7, N, nullptr, false, 1, 0);
1480-
B = DILocation::get(Context, 2, 7, N, nullptr, false, 1, 2);
1483+
A = DILocation::get(Context, 2, 7, N, nullptr, false, /*AtomGroup*/ 1,
1484+
/*AtomRank*/ 0);
1485+
B = DILocation::get(Context, 2, 7, N, nullptr, false, /*AtomGroup*/ 1,
1486+
/*AtomRank*/ 2);
14811487
M = DILocation::getMergedLocation(A, B);
1482-
EXPECT_ATOM(M, 1u, 2u);
1488+
EXPECT_ATOM(M, /*AtomGroup*/ 1u, /*AtomRank*/ 2u);
14831489
EXPECT_EQ(M, DILocation::getMergedLocation(B, A));
14841490
}
14851491

14861492
// Identical but different atom ranks (different atom) - choose the lowest
14871493
// nonzero rank.
14881494
{
1489-
auto *A = DILocation::get(Context, 2, 7, N, nullptr, false, 1, 1);
1490-
auto *B = DILocation::get(Context, 2, 7, N, nullptr, false, 2, 2);
1495+
auto *A = DILocation::get(Context, 2, 7, N, nullptr, false, /*AtomGroup*/ 1,
1496+
/*AtomRank*/ 1);
1497+
auto *B = DILocation::get(Context, 2, 7, N, nullptr, false, /*AtomGroup*/ 2,
1498+
/*AtomRank*/ 2);
14911499
auto *M = DILocation::getMergedLocation(A, B);
14921500
EXPECT_ATOM(M, 1u, 1u);
14931501
EXPECT_EQ(M, DILocation::getMergedLocation(B, A));
14941502

1495-
A = DILocation::get(Context, 2, 7, N, nullptr, false, 1, 0);
1496-
B = DILocation::get(Context, 2, 7, N, nullptr, false, 2, 2);
1503+
A = DILocation::get(Context, 2, 7, N, nullptr, false, /*AtomGroup*/ 1,
1504+
/*AtomRank*/ 0);
1505+
B = DILocation::get(Context, 2, 7, N, nullptr, false, /*AtomGroup*/ 2,
1506+
/*AtomRank*/ 2);
14971507
M = DILocation::getMergedLocation(A, B);
1498-
EXPECT_ATOM(M, 2u, 2u);
1508+
EXPECT_ATOM(M, /*AtomGroup*/ 2u, /*AtomRank*/ 2u);
14991509
EXPECT_EQ(M, DILocation::getMergedLocation(B, A));
15001510
}
15011511

15021512
// Identical but equal atom rank (different atom) - choose the lowest non-zero
15031513
// group (arbitrary choice for deterministic behaviour).
15041514
{
1505-
auto *A = DILocation::get(Context, 2, 7, N, nullptr, false, 1, 1);
1506-
auto *B = DILocation::get(Context, 2, 7, N, nullptr, false, 2, 1);
1515+
auto *A = DILocation::get(Context, 2, 7, N, nullptr, false, /*AtomGroup*/ 1,
1516+
/*AtomRank*/ 1);
1517+
auto *B = DILocation::get(Context, 2, 7, N, nullptr, false, /*AtomGroup*/ 2,
1518+
/*AtomRank*/ 1);
15071519
auto *M = DILocation::getMergedLocation(A, B);
15081520
EXPECT_ATOM(M, 1u, 1u);
15091521
EXPECT_EQ(M, DILocation::getMergedLocation(B, A));
15101522

1511-
A = DILocation::get(Context, 2, 7, N, nullptr, false, 0, 1);
1512-
B = DILocation::get(Context, 2, 7, N, nullptr, false, 2, 1);
1523+
A = DILocation::get(Context, 2, 7, N, nullptr, false, /*AtomGroup*/ 0,
1524+
/*AtomRank*/ 1);
1525+
B = DILocation::get(Context, 2, 7, N, nullptr, false, /*AtomGroup*/ 2,
1526+
/*AtomRank*/ 1);
15131527
M = DILocation::getMergedLocation(A, B);
1514-
EXPECT_ATOM(M, 2u, 1u);
1528+
EXPECT_ATOM(M, /*AtomGroup*/ 2u, /*AtomRank*/ 1u);
15151529
EXPECT_EQ(M, DILocation::getMergedLocation(B, A));
15161530
}
15171531

15181532
// Completely different except same atom numbers. Zero out the atoms.
15191533
{
15201534
auto *I = DILocation::get(Context, 2, 7, N);
1521-
auto *A = DILocation::get(Context, 1, 6, S, I, false, 1, 1);
1522-
auto *B =
1523-
DILocation::get(Context, 2, 7, getSubprogram(), nullptr, false, 1, 1);
1535+
auto *A = DILocation::get(Context, 1, 6, S, I, false, /*AtomGroup*/ 1,
1536+
/*AtomRank*/ 1);
1537+
auto *B = DILocation::get(Context, 2, 7, getSubprogram(), nullptr, false,
1538+
/*AtomGroup*/ 1, /*AtomRank*/ 1);
15241539
auto *M = DILocation::getMergedLocation(A, B);
15251540
EXPECT_EQ(0u, M->getLine());
15261541
EXPECT_EQ(0u, M->getColumn());
@@ -1534,16 +1549,20 @@ TEST_F(DILocationTest, Merge) {
15341549
{
15351550
auto *I = DILocation::get(Context, 1, 7, N);
15361551
auto *F = getSubprogram();
1537-
auto *A = DILocation::get(Context, 1, 1, F, I, false, 1, 2);
1538-
auto *B = DILocation::get(Context, 1, 1, F, I, false, 2, 1);
1552+
auto *A = DILocation::get(Context, 1, 1, F, I, false, /*AtomGroup*/ 1,
1553+
/*AtomRank*/ 2);
1554+
auto *B = DILocation::get(Context, 1, 1, F, I, false, /*AtomGroup*/ 2,
1555+
/*AtomRank*/ 1);
15391556
auto *M = DILocation::getMergedLocation(A, B);
1540-
EXPECT_ATOM(M, 2u, 1u);
1557+
EXPECT_ATOM(M, 2u, /*AtomRank*/ 1u);
15411558
EXPECT_EQ(M, DILocation::getMergedLocation(B, A));
15421559

1543-
A = DILocation::get(Context, 1, 1, F, I, false, 1, 2);
1544-
B = DILocation::get(Context, 1, 1, F, I, false, 2, 0);
1560+
A = DILocation::get(Context, 1, 1, F, I, false, /*AtomGroup*/ 1,
1561+
/*AtomRank*/ 2);
1562+
B = DILocation::get(Context, 1, 1, F, I, false, /*AtomGroup*/ 2,
1563+
/*AtomRank*/ 0);
15451564
M = DILocation::getMergedLocation(A, B);
1546-
EXPECT_ATOM(M, 1u, 2u);
1565+
EXPECT_ATOM(M, /*AtomGroup*/ 1u, /*AtomRank*/ 2u);
15471566
EXPECT_EQ(M, DILocation::getMergedLocation(B, A));
15481567
}
15491568

@@ -1559,24 +1578,25 @@ TEST_F(DILocationTest, Merge) {
15591578
auto *FY = getSubprogram();
15601579
auto *FZ = getSubprogram();
15611580
auto *Z4 = DILocation::get(Context, 1, 4, FZ);
1562-
auto *Y3IntoZ4 = DILocation::get(Context, 1, 3, FY, Z4, false, 1, 1);
1581+
auto *Y3IntoZ4 = DILocation::get(Context, 1, 3, FY, Z4, false,
1582+
/*AtomGroup*/ 1, /*AtomRank*/ 1);
15631583
auto *Y2IntoZ4 = DILocation::get(Context, 1, 2, FY, Z4);
15641584
auto *X1IntoY2 = DILocation::get(Context, 1, 1, FX, Y2IntoZ4);
15651585
auto *M = DILocation::getMergedLocation(X1IntoY2, Y3IntoZ4);
15661586
EXPECT_EQ(M->getScope(), FY);
15671587
EXPECT_EQ(M->getInlinedAt()->getScope(), FZ);
1568-
EXPECT_ATOM(M, 2u, 1u);
1588+
EXPECT_ATOM(M, /*AtomGroup*/ 2u, /*AtomRank*/ 1u);
15691589

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

15731593
// Same again, even if the atom numbers match.
1574-
auto *X1IntoY2SameAtom =
1575-
DILocation::get(Context, 1, 1, FX, Y2IntoZ4, false, 1, 1);
1594+
auto *X1IntoY2SameAtom = DILocation::get(Context, 1, 1, FX, Y2IntoZ4, false,
1595+
/*AtomGroup*/ 1, /*AtomRank*/ 1);
15761596
M = DILocation::getMergedLocation(X1IntoY2SameAtom, Y3IntoZ4);
1577-
EXPECT_ATOM(M, 4u, 1u);
1597+
EXPECT_ATOM(M, /*AtomGroup*/ 4u, /*AtomRank*/ 1u);
15781598
M = DILocation::getMergedLocation(Y3IntoZ4, X1IntoY2SameAtom);
1579-
EXPECT_ATOM(M, 5u, 1u);
1599+
EXPECT_ATOM(M, /*AtomGroup*/ 5u, /*AtomRank*/ 1u);
15801600
}
15811601
#undef EXPECT_ATOM
15821602
}

0 commit comments

Comments
 (0)