[LLVMdev] teaching FileCheck to handle variations in order

Matthew Curtis mcurtis at codeaurora.org
Fri Sep 7 13:55:38 PDT 2012


On 9/7/2012 12:12 PM, Krzysztof Parzyszek wrote:
> On 9/7/2012 7:20 AM, Matthew Curtis wrote:
>>
>> The attached patch implements one possible solution. It introduces a
>> position stack and a couple of directives:
>>
>>   * 'CHECK-PUSH:' pushes the current match position onto the stack.
>>   * 'CHECK-POP:' pops the top value off of the stack and uses it to set
>>     the current match position.
>>
>> The above test can now be re-written as:
>>
>>     ; CHECK-PUSH:
>>     ; CHECK: memw(##a)
>>     ; CHECK-POP:
>>     ; CHECK: memw(##b)
>>
>>     %0 = load i32* @a, align 4
>>     %1 = load i32* @b, align 4
>>
>> which handles either ordering of memory reads for 'a' and 'b'.
>>
>> Thoughts?
>
> I'm not sure if I got the details of how the tests work, but wouldn't 
> this allow false positives?
>
> Take this, for example:
>
> ; CHECK-PUSH:
> ; CHECK: memw(##a)
> ; CHECK-POP:
> ; CHECK: memw(##b)
>
> %0 = load i32* @a, align 4
> %1 = load i32* @b, align 4
>
> ; CHECK: memw(##a)
>
> %2 = load i32* @a, align 4
>
>
> My understanding is that this is supposed to detect:
> - 2 loads in any order, one from @a, the other from @b, followed by
> - 1 load from @a,
> that is, a total of 3 loads.
>
> At the same time I believe that it would positively match this code:
>   ... = memw(##b)
>   ... = memw(##a)
> i.e. only two loads.  The second load would match two different CHECKs.
>
> -K
>
You are correct. If the intent is to detect:

  * 2 loads in any order, one from @a, the other from @b, followed by
  * 1 load from @a,

the example does not do that. It matches additional sequences as well.

I don't believe the PUSH/POP functionality is expressive enough to 
handle this case. You would have to modify the test in some way or add 
more functionality to FileCheck to handle this.

Thinking of possible solutions ...

We could keep track of a "high water mark" and add a directive to set 
the position to it:

    ; CHECK-PUSH:
    ; CHECK: memw(##a)
    ; CHECK-POP:
    ; CHECK: memw(##b)
    ; CHECK-HWM:

    %0 = load i32* @a, align 4
    %1 = load i32* @b, align 4

    ; CHECK: memw(##a)

    %2 = load i32* @a, align 4

In this case CHECK-HWM (need a better name) will set the position to 
just after 'memw(##a)' or 'memw(##b)' whichever is later in the file.

I've attached a new patch with support for the additional directive.

Matthew Curtis.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20120907/c61e1aaf/attachment.html>
-------------- next part --------------
diff --git a/utils/FileCheck/FileCheck.cpp b/utils/FileCheck/FileCheck.cpp
index 33f04ce..b6c4f51 100644
--- a/utils/FileCheck/FileCheck.cpp
+++ b/utils/FileCheck/FileCheck.cpp
@@ -50,11 +50,23 @@ NoCanonicalizeWhiteSpace("strict-whitespace",
 //===----------------------------------------------------------------------===//
 
 class Pattern {
+public:
+  enum MatchType {
+    MatchStr,
+    MatchCurrent,
+    MatchEndOfFile
+  };
+
+private:
   SMLoc PatternLoc;
 
-  /// MatchEOF - When set, this pattern only matches the end of file. This is
-  /// used for trailing CHECK-NOTs.
-  bool MatchEOF;
+  /// MatchType - When set to ...
+  ///   MatchStr, this pattern matches according to FixedStr or RegExStr.
+  ///   MatchCurrent, this pattern matches (the empty string at) the current
+  ///     position. This is used for CHECK-PUSHes without preceding CHECKs.
+  ///   MatchEndOfFile, this pattern only matches the end of file. This is used
+  ///     for trailing CHECK-NOTs.
+  enum MatchType Type;
 
   /// FixedStr - If non-empty, this pattern is a fixed string match with the
   /// specified fixed string.
@@ -77,7 +89,7 @@ class Pattern {
 
 public:
 
-  Pattern(bool matchEOF = false) : MatchEOF(matchEOF) { }
+  Pattern(enum MatchType t = MatchStr) : Type(t) { }
 
   bool ParsePattern(StringRef PatternStr, SourceMgr &SM);
 
@@ -285,9 +297,18 @@ bool Pattern::AddRegExToRegEx(StringRef RegexStr, unsigned &CurParen,
 size_t Pattern::Match(StringRef Buffer, size_t &MatchLen,
                       StringMap<StringRef> &VariableTable) const {
   // If this is the EOF pattern, match it immediately.
-  if (MatchEOF) {
+  switch (Type) {
+  case MatchStr:
+    break;
+  case MatchCurrent:
+    MatchLen = 0;
+    return 0;
+  case MatchEndOfFile:
     MatchLen = 0;
     return Buffer.size();
+  default:
+    assert("Unknown match type");
+    break;
   }
 
   // If this is a fixed string pattern, just match it now.
@@ -447,6 +468,9 @@ struct CheckString {
   /// IsCheckNext - This is true if this is a CHECK-NEXT: directive (as opposed
   /// to a CHECK: directive.
   bool IsCheckNext;
+  int PushPos;
+  int PopPos;
+  SMLoc PopLoc;
 
   /// NotStrings - These are all of the strings that are disallowed from
   /// occurring between this match string and the previous one (or start of
@@ -454,7 +478,7 @@ struct CheckString {
   std::vector<std::pair<SMLoc, Pattern> > NotStrings;
 
   CheckString(const Pattern &P, SMLoc L, bool isCheckNext)
-    : Pat(P), Loc(L), IsCheckNext(isCheckNext) {}
+    : Pat(P), Loc(L), IsCheckNext(isCheckNext), PushPos(0), PopPos(0) {}
 };
 
 /// CanonicalizeInputFile - Remove duplicate horizontal space from the specified
@@ -517,6 +541,8 @@ static bool ReadCheckFile(SourceMgr &SM,
   StringRef Buffer = F->getBuffer();
 
   std::vector<std::pair<SMLoc, Pattern> > NotMatches;
+  int NextCheckPopPos= 0;
+  SMLoc NextCheckPopLoc;
 
   while (1) {
     // See if Prefix occurs in the memory buffer.
@@ -531,6 +557,7 @@ static bool ReadCheckFile(SourceMgr &SM,
     // When we find a check prefix, keep track of whether we find CHECK: or
     // CHECK-NEXT:
     bool IsCheckNext = false, IsCheckNot = false;
+    bool IsCheckPush = false, IsCheckPop = false, IsCheckHWM = false;
 
     // Verify that the : is present after the prefix.
     if (Buffer[CheckPrefix.size()] == ':') {
@@ -543,6 +570,18 @@ static bool ReadCheckFile(SourceMgr &SM,
                memcmp(Buffer.data()+CheckPrefix.size(), "-NOT:", 5) == 0) {
       Buffer = Buffer.substr(CheckPrefix.size()+6);
       IsCheckNot = true;
+    } else if (Buffer.size() > CheckPrefix.size()+6 &&
+               memcmp(Buffer.data()+CheckPrefix.size(), "-PUSH:", 6) == 0) {
+      Buffer = Buffer.substr(CheckPrefix.size()+7);
+      IsCheckPush = true;
+    } else if (Buffer.size() > CheckPrefix.size()+5 &&
+               memcmp(Buffer.data()+CheckPrefix.size(), "-POP:", 5) == 0) {
+      Buffer = Buffer.substr(CheckPrefix.size()+6);
+      IsCheckPop = true;
+    } else if (Buffer.size() > CheckPrefix.size()+5 &&
+               memcmp(Buffer.data()+CheckPrefix.size(), "-HWM:", 5) == 0) {
+      Buffer = Buffer.substr(CheckPrefix.size()+6);
+      IsCheckHWM = true;
     } else {
       Buffer = Buffer.substr(1);
       continue;
@@ -555,6 +594,34 @@ static bool ReadCheckFile(SourceMgr &SM,
     // Scan ahead to the end of line.
     size_t EOL = Buffer.find_first_of("\n\r");
 
+    if (IsCheckPush) {
+      if (CheckStrings.empty()) {
+        CheckStrings.push_back(CheckString(Pattern(Pattern::MatchCurrent),
+                                           SMLoc::getFromPointer(Buffer.data()),
+                                           false));
+      }
+      CheckStrings.back().PushPos++;
+      continue;
+    }
+
+    if (IsCheckPop) {
+      NextCheckPopLoc = SMLoc::getFromPointer(CheckPrefixStart);
+      NextCheckPopPos++;
+      continue;
+    }
+
+    if (IsCheckHWM) {
+      if (NextCheckPopPos) {
+        SM.PrintMessage(SMLoc::getFromPointer(CheckPrefixStart),
+                        SourceMgr::DK_Error,
+                        "found '"+CheckPrefix+"-HWM:' without previous '"+
+                        CheckPrefix+ ": line");
+      }
+      NextCheckPopLoc = SMLoc::getFromPointer(CheckPrefixStart);
+      NextCheckPopPos = -1;
+      continue;
+    }
+
     // Remember the location of the start of the pattern, for diagnostics.
     SMLoc PatternLoc = SMLoc::getFromPointer(Buffer.data());
 
@@ -587,14 +654,22 @@ static bool ReadCheckFile(SourceMgr &SM,
     CheckStrings.push_back(CheckString(P,
                                        PatternLoc,
                                        IsCheckNext));
+    CheckStrings.back().PopPos= NextCheckPopPos;
+    CheckStrings.back().PopLoc= NextCheckPopLoc;
+    NextCheckPopPos= 0;
+    NextCheckPopLoc= SMLoc();
     std::swap(NotMatches, CheckStrings.back().NotStrings);
   }
 
   // Add an EOF pattern for any trailing CHECK-NOTs.
   if (!NotMatches.empty()) {
-    CheckStrings.push_back(CheckString(Pattern(true),
+    CheckStrings.push_back(CheckString(Pattern(Pattern::MatchEndOfFile),
                                        SMLoc::getFromPointer(Buffer.data()),
                                        false));
+    CheckStrings.back().PopPos= NextCheckPopPos;
+    CheckStrings.back().PopLoc= NextCheckPopLoc;
+    NextCheckPopPos= 0;
+    NextCheckPopLoc= SMLoc();
     std::swap(NotMatches, CheckStrings.back().NotStrings);
   }
 
@@ -686,10 +761,32 @@ int main(int argc, char **argv) {
   StringRef Buffer = F->getBuffer();
 
   const char *LastMatch = Buffer.data();
+  std::vector<StringRef> BufferStack;
+  StringRef BufferHighWaterMark = Buffer;
 
   for (unsigned StrNo = 0, e = CheckStrings.size(); StrNo != e; ++StrNo) {
     const CheckString &CheckStr = CheckStrings[StrNo];
 
+    if (CheckStr.PopPos == -1) {
+      if (Buffer.data() < BufferHighWaterMark.data()) {
+        Buffer = BufferHighWaterMark;
+        LastMatch = Buffer.data();
+      }
+    } else {
+      for (int i = CheckStr.PopPos; i; --i) {
+        if (BufferStack.empty()) {
+          SM.PrintMessage(CheckStr.PopLoc, SourceMgr::DK_Error,
+                          "attempting to '"+CheckPrefix+"-POP:' without previous '"+
+                          CheckPrefix+ "-PUSH:'");
+          return 1;
+        }
+        if (Buffer.data() > BufferHighWaterMark.data())
+          BufferHighWaterMark = Buffer;
+        Buffer = BufferStack.back();
+        BufferStack.pop_back();
+      }
+    }
+
     StringRef SearchFrom = Buffer;
 
     // Find StrNo in the file.
@@ -756,6 +853,9 @@ int main(int argc, char **argv) {
     // the position after the match as the end of the last match.
     Buffer = Buffer.substr(MatchLen);
     LastMatch = Buffer.data();
+    for (int i= CheckStr.PushPos; i; --i) {
+      BufferStack.push_back(Buffer);
+    }
   }
 
   return 0;


More information about the llvm-dev mailing list