Skip to content
This repository was archived by the owner on Feb 5, 2019. It is now read-only.

Commit a46c1d6

Browse files
committed
[FileCheck] Fix search ranges for DAG-NOT-DAG
A DAG-NOT-DAG is a CHECK-DAG group, X, followed by a CHECK-NOT group, N, followed by a CHECK-DAG group, Y. Let y be the initial directive of Y. This patch makes the following changes to the behavior: 1. Directives in N can no longer match within part of Y's match range just because y happens not to be the earliest match from Y. Specifically, this patch withdraws N's search range end from y's match range start to Y's match range start. 2. y can no longer match within X's match range, where a y match produced a reordering complaint, which is thus no longer possible. Specifically, this patch withdraws y's search range start from X's permitted range start to X's match range end, which was already the search range start for other members of Y. Both of these changes can only increase the number of test passes: #1 constrains the ability of CHECK-NOTs to match, and #2 expands the ability of CHECK-DAGs to match without complaints. These changes are based on discussions at: <http://lists.llvm.org/pipermail/llvm-dev/2018-May/123550.html> <https://reviews.llvm.org/D47106> which conclude that: 1. These changes simplify the FileCheck conceptual model. First, it makes search ranges for DAG-NOT-DAG more consistent with other cases. Second, it was confusing that y was treated differently from the rest of Y. 2. These changes add theoretical use cases for DAG-NOT-DAG that had no obvious means to be expressed otherwise. We can justify the first half of this assertion with the observation that these changes can only increase the number of test passes. 3. Reordering detection for DAG-NOT-DAG had no obvious real benefit. We don't have evidence from real uses cases to help us debate conclusions #2 and #3, but #1 at least seems intuitive. Reviewed By: probinson Differential Revision: https://reviews.llvm.org/D48986 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@337605 91177308-0d34-0410-b5e6-96231b3b80d8
1 parent ff83113 commit a46c1d6

File tree

3 files changed

+124
-47
lines changed

3 files changed

+124
-47
lines changed

test/FileCheck/check-dag-not-dag.txt

+76
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
;---------------------------------------------------------------------
2+
; RUN: FileCheck -input-file %s %s -check-prefix=NotSearchEnd
3+
4+
The search range for the NOTs used to end at the start of the match range for
5+
the first DAG in the following DAG group. Now it ends at the start of the
6+
match range for the entire following DAG group.
7+
8+
__NotSearchEnd
9+
x0
10+
x1
11+
12+
y1
13+
foobar
14+
y0
15+
16+
z2
17+
foobar
18+
z1
19+
foobar
20+
z0
21+
__NotSearchEnd
22+
23+
; NotSearchEnd: {{^}}__NotSearchEnd
24+
; NotSearchEnd-DAG: {{^}}x0
25+
; NotSearchEnd-DAG: {{^}}x1
26+
; NotSearchEnd-NOT: {{^}}foobar
27+
; NotSearchEnd-DAG: {{^}}y0
28+
; NotSearchEnd-DAG: {{^}}y1
29+
; NotSearchEnd-NOT: {{^}}foobar
30+
; NotSearchEnd-DAG: {{^}}z0
31+
; NotSearchEnd-DAG: {{^}}z1
32+
; NotSearchEnd-DAG: {{^}}z2
33+
; NotSearchEnd: {{^}}__NotSearchEnd
34+
35+
;---------------------------------------------------------------------
36+
; RUN: FileCheck -input-file %s %s -check-prefix=Dag2SearchStart
37+
38+
The start of the search range for the second or later DAG group used to be
39+
different for its first DAG than its other DAGs. For the first DAG, it was
40+
the start of the permitted range for the preceding DAG group, and there was a
41+
reordering complaint if the match range was in the first DAG group's match
42+
range. For the other DAGs, it was the end of the match range for the
43+
preceding DAG group, so reordering detection wasn't possible. Now, the
44+
first DAG behaves like the others, and so reordering detection is no longer
45+
implemented. As a result, matches that used to produce the reordering
46+
complaint are now skipped, permitting later matches to succeed.
47+
48+
__Dag2SearchStart
49+
y0
50+
y1
51+
x0
52+
y0
53+
y1
54+
x1
55+
56+
z1
57+
z0
58+
y1
59+
z1
60+
z0
61+
y0
62+
63+
z0
64+
z1
65+
__Dag2SearchStart
66+
67+
; Dag2SearchStart: {{^}}__Dag2SearchStart
68+
; Dag2SearchStart-DAG: {{^}}x0
69+
; Dag2SearchStart-DAG: {{^}}x1
70+
; Dag2SearchStart-NOT: {{^}}foobar
71+
; Dag2SearchStart-DAG: {{^}}y0
72+
; Dag2SearchStart-DAG: {{^}}y1
73+
; Dag2SearchStart-NOT: {{^}}foobar
74+
; Dag2SearchStart-DAG: {{^}}z0
75+
; Dag2SearchStart-DAG: {{^}}z1
76+
; Dag2SearchStart: {{^}}__Dag2SearchStart

test/FileCheck/check-dag-overlap.txt

+2-1
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,8 @@ Assume we have DAGs, NOTs, DAGs, NOTs, and then DAGs. Let X, Y, and Z be
168168
the DAG groups such that the leading DAGs are x, y, and z. y won't match
169169
overlaps with matches from:
170170

171-
1. X. Otherwise, we could get a spurious reordering complaint.
171+
1. X. Otherwise, we used to get a spurious reordering complaint (back
172+
when reordering complaints on DAG-NOT-DAG were still implemented).
172173
2. Y, because y is in Y. To prevent these overlaps, the implementation must be
173174
careful not to drop y's match from the previous matches list when it drops
174175
matches from X to save search time.

utils/FileCheck/FileCheck.cpp

+46-46
Original file line numberDiff line numberDiff line change
@@ -1283,17 +1283,23 @@ size_t CheckString::CheckDag(const SourceMgr &SM, StringRef Buffer,
12831283
if (DagNotStrings.empty())
12841284
return 0;
12851285

1286-
size_t LastPos = 0;
1287-
size_t StartPos = LastPos;
1286+
// The start of the search range.
1287+
size_t StartPos = 0;
12881288

1289-
// A sorted list of ranges for non-overlapping dag matches.
1290-
struct Match {
1289+
struct MatchRange {
12911290
size_t Pos;
12921291
size_t End;
12931292
};
1294-
std::list<Match> Matches;
1295-
1296-
for (const Pattern &Pat : DagNotStrings) {
1293+
// A sorted list of ranges for non-overlapping CHECK-DAG matches. Match
1294+
// ranges are erased from this list once they are no longer in the search
1295+
// range.
1296+
std::list<MatchRange> MatchRanges;
1297+
1298+
// We need PatItr and PatEnd later for detecting the end of a CHECK-DAG
1299+
// group, so we don't use a range-based for loop here.
1300+
for (auto PatItr = DagNotStrings.begin(), PatEnd = DagNotStrings.end();
1301+
PatItr != PatEnd; ++PatItr) {
1302+
const Pattern &Pat = *PatItr;
12971303
assert((Pat.getCheckTy() == Check::CheckDAG ||
12981304
Pat.getCheckTy() == Check::CheckNot) &&
12991305
"Invalid CHECK-DAG or CHECK-NOT!");
@@ -1310,7 +1316,7 @@ size_t CheckString::CheckDag(const SourceMgr &SM, StringRef Buffer,
13101316

13111317
// Search for a match that doesn't overlap a previous match in this
13121318
// CHECK-DAG group.
1313-
for (auto MI = Matches.begin(), ME = Matches.end(); true; ++MI) {
1319+
for (auto MI = MatchRanges.begin(), ME = MatchRanges.end(); true; ++MI) {
13141320
StringRef MatchBuffer = Buffer.substr(MatchPos);
13151321
size_t MatchPosBuf = Pat.Match(MatchBuffer, MatchLen, VariableTable);
13161322
// With a group of CHECK-DAGs, a single mismatching means the match on
@@ -1325,10 +1331,21 @@ size_t CheckString::CheckDag(const SourceMgr &SM, StringRef Buffer,
13251331
if (VerboseVerbose)
13261332
PrintMatch(true, SM, Prefix, Pat.getLoc(), Pat, Buffer, VariableTable,
13271333
MatchPos, MatchLen);
1328-
if (AllowDeprecatedDagOverlap)
1334+
MatchRange M{MatchPos, MatchPos + MatchLen};
1335+
if (AllowDeprecatedDagOverlap) {
1336+
// We don't need to track all matches in this mode, so we just maintain
1337+
// one match range that encompasses the current CHECK-DAG group's
1338+
// matches.
1339+
if (MatchRanges.empty())
1340+
MatchRanges.insert(MatchRanges.end(), M);
1341+
else {
1342+
auto Block = MatchRanges.begin();
1343+
Block->Pos = std::min(Block->Pos, M.Pos);
1344+
Block->End = std::max(Block->End, M.End);
1345+
}
13291346
break;
1347+
}
13301348
// Iterate previous matches until overlapping match or insertion point.
1331-
Match M{MatchPos, MatchPos + MatchLen};
13321349
bool Overlap = false;
13331350
for (; MI != ME; ++MI) {
13341351
if (M.Pos < MI->End) {
@@ -1340,7 +1357,7 @@ size_t CheckString::CheckDag(const SourceMgr &SM, StringRef Buffer,
13401357
}
13411358
if (!Overlap) {
13421359
// Insert non-overlapping match into list.
1343-
Matches.insert(MI, M);
1360+
MatchRanges.insert(MI, M);
13441361
break;
13451362
}
13461363
if (VerboseVerbose) {
@@ -1357,46 +1374,29 @@ size_t CheckString::CheckDag(const SourceMgr &SM, StringRef Buffer,
13571374
PrintMatch(true, SM, Prefix, Pat.getLoc(), Pat, Buffer, VariableTable,
13581375
MatchPos, MatchLen);
13591376

1360-
if (!NotStrings.empty()) {
1361-
if (MatchPos < LastPos) {
1362-
// Reordered?
1363-
SM.PrintMessage(SMLoc::getFromPointer(Buffer.data() + MatchPos),
1364-
SourceMgr::DK_Error,
1365-
Prefix + "-DAG: found a match of CHECK-DAG"
1366-
" reordering across a CHECK-NOT");
1367-
SM.PrintMessage(SMLoc::getFromPointer(Buffer.data() + LastPos),
1368-
SourceMgr::DK_Note,
1369-
Prefix + "-DAG: the farthest match of CHECK-DAG"
1370-
" is found here");
1371-
SM.PrintMessage(NotStrings[0]->getLoc(), SourceMgr::DK_Note,
1372-
Prefix + "-NOT: the crossed pattern specified"
1373-
" here");
1374-
SM.PrintMessage(Pat.getLoc(), SourceMgr::DK_Note,
1375-
Prefix + "-DAG: the reordered pattern specified"
1376-
" here");
1377-
return StringRef::npos;
1377+
// Handle the end of a CHECK-DAG group.
1378+
if (std::next(PatItr) == PatEnd ||
1379+
std::next(PatItr)->getCheckTy() == Check::CheckNot) {
1380+
if (!NotStrings.empty()) {
1381+
// If there are CHECK-NOTs between two CHECK-DAGs or from CHECK to
1382+
// CHECK-DAG, verify that there are no 'not' strings occurred in that
1383+
// region.
1384+
StringRef SkippedRegion =
1385+
Buffer.slice(StartPos, MatchRanges.begin()->Pos);
1386+
if (CheckNot(SM, SkippedRegion, NotStrings, VariableTable))
1387+
return StringRef::npos;
1388+
// Clear "not strings".
1389+
NotStrings.clear();
13781390
}
1379-
// All subsequent CHECK-DAGs should be matched from the farthest
1380-
// position of all precedent CHECK-DAGs (not including this one).
1381-
StartPos = LastPos;
1391+
// All subsequent CHECK-DAGs and CHECK-NOTs should be matched from the
1392+
// end of this CHECK-DAG group's match range.
1393+
StartPos = MatchRanges.rbegin()->End;
13821394
// Don't waste time checking for (impossible) overlaps before that.
1383-
Matches.clear();
1384-
Matches.push_back(Match{MatchPos, MatchPos + MatchLen});
1385-
// If there's CHECK-NOTs between two CHECK-DAGs or from CHECK to
1386-
// CHECK-DAG, verify that there's no 'not' strings occurred in that
1387-
// region.
1388-
StringRef SkippedRegion = Buffer.slice(LastPos, MatchPos);
1389-
if (CheckNot(SM, SkippedRegion, NotStrings, VariableTable))
1390-
return StringRef::npos;
1391-
// Clear "not strings".
1392-
NotStrings.clear();
1395+
MatchRanges.clear();
13931396
}
1394-
1395-
// Update the last position with CHECK-DAG matches.
1396-
LastPos = std::max(MatchPos + MatchLen, LastPos);
13971397
}
13981398

1399-
return LastPos;
1399+
return StartPos;
14001400
}
14011401

14021402
// A check prefix must contain only alphanumeric, hyphens and underscores.

0 commit comments

Comments
 (0)