[FileCheck] Add function call support to numerical expressions.

[paulwalker-arm]

Summary

This patch extends numerical expressions to allow calls to
predefined functions. These calls can be combined with the
existing numerical operators, which includes nesting calls.

The call syntax is:

  <func>(<args>)

Where <func> is a predefined string literal, currently limited to
one of add, max, min and sub. <arg> is a comma seperated list of
numerical expressions.

[paulwalker-arm]

This is very much work in progress but I welcome early feedback.

I don't know if a function name prefix is necessary but at this stage is allows me to ignore some corner cases. I'd also like to know whether I can get away with only supporting arbitrary argument counts at the parsing layer since currently I only need support for the usual two operand math operations.

[paulwalker-arm]

Only a minor code change so still WIP, but with basic tests and documentation update.

[arichardson]

I like this approach. Starting functions with ! seems reasonable since it is similar tablegen and if we decide that we don't need the prefix, we can always drop it.
The advantage of requiring the prefix is that we can give functions names that might also be commonly used capture names.

There should probably be some tests for error messages in llvm/unittests/Support/FileCheckTest.cpp.

[paulwalker-arm]

I believe the patch is now ready for review. There's an open question as to
whether the precedence operator (i.e. !()) is required, but since it's
implementation came largely for free I ran with it.

[paulwalker-arm]

Baseline update and fixed naming issue reported by clang-tidy.

[paulwalker-arm]

Added udiv to complement mul.

[paulwalker-arm]

I don't know if there's an official mechanism beyond adding people but can I request code review please.

[thopre]

Please note that the patch to add support for signed values (https://reviews.llvm.org/D60390) is at an advanced stage of review.

llvm/docs/CommandGuide/FileCheck.rst

693

Why change this sentence? Recursion only happens on one of the operand only.

725–726

I don't think the exclamation mark should be required here. A parenthesis pair should be enough to force precedence. Note that there was a patch to adds support for that and you might want to rebase your patch on top of it.

llvm/lib/Support/FileCheck.cpp

646–649

Please group the two tests together, together they test whether it's an exit condition.

[thopre]

In D79936#2054735, @paulwalker-arm wrote:

I don't know if there's an official mechanism beyond adding people but can I request code review please.

Adding people as reviewer is the official way. The review policy [1] also says you can ping after 1 week if you didn't have any reply.

[1] https://llvm.org/docs/CodeReview.html

[paulwalker-arm]

llvm/docs/CommandGuide/FileCheck.rst

693

I don't understand the distinction. I changed the sentence as I didn't what to say:

an expression followed by an operator and either a numerical operand or a function call.

Are you saying the above is more correct?

Also with functions you can have

!(mul(VAR+(umin(VAR2,4)) +  !(udivl(VAR3+(umax(VAR3,4))

725–726

Sure, it's more that this implementation comes for free, whereas supporting arbitrary parenthesis pairs along side function calls requires more complex parsing. Is it worth the extra effort? If so I'll happy prevent this usage and error out for unnamed functions.

I'd rather not wait for the parenthesis work because I've got other work at review whose tests are much improved when I can make use of function calls.

[thopre]

llvm/docs/CommandGuide/FileCheck.rst

693

Oh right, I forgot about function altogether. That said, I think an expression is a kind of numeric operand so we should just expand the definition below saying it can also be a function call.

[thopre]

llvm/docs/CommandGuide/FileCheck.rst

725–726

I think the exclamation mark should be reserved for function call. I find it a bit confusing otherwise but let's wait to see what other reviewers might think.

Do the tests you need this feature for require some way to force precedence or can this be dealt later?

[arichardson]

llvm/docs/CommandGuide/FileCheck.rst

725–726

Apologies for the delay, I've now rebased the parentheses revision (D77383).

[paulwalker-arm]

llvm/docs/CommandGuide/FileCheck.rst

693

So something like:

A numeric operand is a previously defined numeric variable, an integer literal or the result of a function call.

725–726

I currently have no use for the precedence operator, the function calls are why I started down this path, so am happy either way.

Part of me just assumed that in the future the parser might be reworked to remove the need for the ! prefix, but I suppose providing the power of precedence early might prevent that work from happening :)

[thopre]

llvm/docs/CommandGuide/FileCheck.rst

693

I'm nitpicking but technically the operand is the function call itself, but evaluates to the return value of the function call.

725–726

I wouldn't hold my breath for a parser rewrite. While I'd love to make the code nicer and more flexible I have little free time to work on it myself. Anyway, if you don't require operator precedence just error on empty function name for now and we can extend it to be used for operator precedence later if needed.

[paulwalker-arm]

rebase and post code review fixes

[paulwalker-arm]

llvm/docs/CommandGuide/FileCheck.rst

725–726

Done. Perhaps it's a good idea to mandate that all symbolic operators have a named function counterpart. That way in the short term if somebody does want to force precedence they just need to write a slightly more verbose check line.

[thopre]

llvm/docs/CommandGuide/FileCheck.rst

725–726

No objection to that, should be a 2 line changes, right?

[paulwalker-arm]

llvm/docs/CommandGuide/FileCheck.rst

725–726

Yep. I'll add entries for add and sub to this patch and resubmit later today.

[paulwalker-arm]

Added functions for add and sub.

[paulwalker-arm]

rebase

[arichardson]

LGTM if @thopre and @jhenderson are happy with this change too.

llvm/unittests/Support/FileCheckTest.cpp

1076

Might make sense to add case with missing operators such as 2!mul(FOO,2) or FOO !mul(FOO,2) or !mul(FOO(!mul(3,2)))

[jhenderson]

Some of my testing suggestions might better belong in the unit tests. Also, you're probably going to need to rebase and expand the behaviour somewhat now that signed values support has landed in D60390.

llvm/lib/Support/FileCheck.cpp

486

I'm not strongly opposed to the use of ! to indicate a function call, but is it actually necessary? It seems like a function call could just be identified by <sequence of identifier chars>(<some chars>). The code would look something like the following semi-pseudo code:

size_t Parenthesis = Expr.find_first_of('(');
if (Parenthesis != npos) {
  if (all_of(Expr.take_front(Parenthesis), [](char C) { 
      return isValidIdentifierChar(C);
    }) {
    if (AO != AllowedOperand::Any)
      return ErrorDiagnostic::get(SM, Expr, "unexpected function call");

    return parseCallExpr(Expr, LineNumber, Context, SM);
  }
}

Assuming I've not missed something, that would allow us to simplify all the usages of function calls.

616–617

I believe you could change this case to an asseertion - the parseExpr function treats a leading '(' as a different kind of expression.

693

In conjunction with my suggestion above about not having a function specifier, you could change this code to bail out without error in some cases, perhaps by starting with looking for the % followed by some specific characters, followed by a ,.

llvm/lib/Support/FileCheckImpl.h

735–741

I think you need to delete "both" here, since there are now three different things it accepts.

llvm/test/FileCheck/numeric-expression.txt

120

Perhaps change the inner UNSI to UNSI+1 or something to show that the argument of a function is any kind of expression? Up to you.

384

I would prefer these to be interleaved with their corresponding CHECK and input text:

RUN: ... --check-prefix CALL-MISSING-OPENING-BRACKET ...

CALL MISSING OPENING BRACKET
30
CALL-MISSING-OPENING-BRACKET-LABEL: ...
...

RUN: ... --check-prefix CALL-MISSING-CLOSING-BRACKET ...

CALL MISSING CLOSING BRACKET

etc. It helps reduce the distance I have to look to find the thing being checked for.

409

There might want to be some interaction testing with plain parentheses. Something like [[#!mul(NUMVAR,(NUMVAR+3))]] and [[#!mul(NUMVAR,(NUMVAR+3)]] (the first should work, but not the second).

417

Nit: it would probably be best to make this call take two arguments.

425

I think you also want the following: [[#!mul(,NUMVAR)]]

Possibly also [[#!mul(NUMVAR,,NUMVAR)]]

441

Nit: it would probably be best to make this call take two arguments.

llvm/unittests/Support/FileCheckTest.cpp

1076

+1

[thopre]

llvm/lib/Support/FileCheck.cpp

486

Regardless of the ease of implementation, I like the ! prefix since these are builtin functions/operators, not something the user can define. YMMV of course

[jhenderson]

llvm/lib/Support/FileCheck.cpp

486

I'm not sure why it matters that they are builtin? Even if we do provide the ability for users to define their own functions, surely their behaviour should be identical to built-in functions from the majority of the code's point-of-view? I'd actually think that including the ! would make it harder to parse, since we'd have to support function calls both with and without the !.

[arichardson]

llvm/lib/Support/FileCheck.cpp

486

If we can make it work without the ! without making the implementation much more complicated, I'd prefer that.
But I don't feel strongly either way.

Since the name needs to be followed by an open paren, even variables that have the same name as builtin functions should work: [[#mul(mul, 2)]]. The first one is a function name, the second must be a variable since there is no open paren.

[paulwalker-arm]

Prefix aside I'm just doing another rebase to bring in the signedness work. I'd like to know if it's going to be a requirement to support the reporting of overflow/underflow for the builtins in my patch. Originally I had gone down the llvm route of making the operations signed rather than the data but I see the signedness patch implements the opposite. Ultimately I need to know if there's light at the end of the tunnel or whether to give up and just write ugly tests.

[paulwalker-arm]

Rebased but still need to remove the function prefix and tighen up the mul/div operators.

[paulwalker-arm]

Added the suggested tests to the FileCheck unitest plus fleshed out overflow reporting.

[paulwalker-arm]

I have a clang-format query. I'm getting failures because clang-format is suggesting to use "-<space><digit>" to format a negative number. This doesn't seem correct to me and is not the style I see for existing code in FileCheckTests.cpp. Is this something I can ignore?

[jhenderson]

In D79936#2062802, @paulwalker-arm wrote:

I'd like to know if it's going to be a requirement to support the reporting of overflow/underflow for the builtins in my patch. Originally I had gone down the llvm route of making the operations signed rather than the data but I see the signedness patch implements the opposite. Ultimately I need to know if there's light at the end of the tunnel or whether to give up and just write ugly tests.

(Wrote this comment before I saw you added overflow/underflow support, but leaving it because it might give an idea of my thought process on why): Not quite sure I fully followed this comment. I think my preference would be to error out for overflows/underflows, rather than silently allowing them. If things are going to be significantly more complex adding them but you are also going to address them immediately, I'm okay with it being deferred to a future patch. What I don't want long-term is for people to be able to write unintentionally broken test cases because they happen to be triggering underflow/overflow behaviour. Broken test cases are bad!

In D79936#2064448, @paulwalker-arm wrote:

I have a clang-format query. I'm getting failures because clang-format is suggesting to use "-<space><digit>" to format a negative number. This doesn't seem correct to me and is not the style I see for existing code in FileCheckTests.cpp. Is this something I can ignore?

@thopre ran into this recently too. I consider it a bug in clang-format personally, so you can ignore it, but if @thopre hasn't already, you should file a clang-format bug so that it can get fixed.

llvm/docs/CommandGuide/FileCheck.rst

695

The "and have a 64-bit precision" bit seems a bit out of place here. It should probably be its own sentence.

696

Given the new stuff about functions, I might be tempted to pull out the sentence about supported operators into a list, a bit like that used for the accepted values, especially since it needs updating as things stand!

llvm/lib/Support/FileCheck.cpp

233

I think adding operator* etc makes sense, but it should be a separate patch to adding function support (probably a prerequisite). We don't want to cloud the intent of this patch by adding in other useful functionality, and it will make it easier to focus the reviewing.

I'll save reviewing them for that patch.

317–318

Did you consider writing min in terms of max (or vice versa)? Not sure if it is a good thing to do or not, but I believe it would lead to less duplicated and more concise code. Something like:

if (max(LeftOperand, RightOperand) == LeftOperand)
  return RightOperand;
return LeftOperand;

[paulwalker-arm]

llvm/lib/Support/FileCheck.cpp

233

I've created D80915 to add the new operator functions.

[thopre]

In D79936#2065837, @jhenderson wrote:

In D79936#2062802, @paulwalker-arm wrote:

I'd like to know if it's going to be a requirement to support the reporting of overflow/underflow for the builtins in my patch. Originally I had gone down the llvm route of making the operations signed rather than the data but I see the signedness patch implements the opposite. Ultimately I need to know if there's light at the end of the tunnel or whether to give up and just write ugly tests.

(Wrote this comment before I saw you added overflow/underflow support, but leaving it because it might give an idea of my thought process on why): Not quite sure I fully followed this comment. I think my preference would be to error out for overflows/underflows, rather than silently allowing them. If things are going to be significantly more complex adding them but you are also going to address them immediately, I'm okay with it being deferred to a future patch. What I don't want long-term is for people to be able to write unintentionally broken test cases because they happen to be triggering underflow/overflow behaviour. Broken test cases are bad!

In D79936#2064448, @paulwalker-arm wrote:

I have a clang-format query. I'm getting failures because clang-format is suggesting to use "-<space><digit>" to format a negative number. This doesn't seem correct to me and is not the style I see for existing code in FileCheckTests.cpp. Is this something I can ignore?

@thopre ran into this recently too. I consider it a bug in clang-format personally, so you can ignore it, but if @thopre hasn't already, you should file a clang-format bug so that it can get fixed.

I'm planning to take a look at this clang-format bug today.

[thopre]

In D79936#2062802, @paulwalker-arm wrote:

Prefix aside I'm just doing another rebase to bring in the signedness work. I'd like to know if it's going to be a requirement to support the reporting of overflow/underflow for the builtins in my patch. Originally I had gone down the llvm route of making the operations signed rather than the data but I see the signedness patch implements the opposite. Ultimately I need to know if there's light at the end of the tunnel or whether to give up and just write ugly tests.

Using signed operation would mean throwing overflow when the result could be represented in uint64_t in some cases which felt weird, especially since we support unsigned values (e.g. addresses).

[thopre]

In D79936#2066160, @thopre wrote:

In D79936#2065837, @jhenderson wrote:

In D79936#2062802, @paulwalker-arm wrote:

I'd like to know if it's going to be a requirement to support the reporting of overflow/underflow for the builtins in my patch. Originally I had gone down the llvm route of making the operations signed rather than the data but I see the signedness patch implements the opposite. Ultimately I need to know if there's light at the end of the tunnel or whether to give up and just write ugly tests.

(Wrote this comment before I saw you added overflow/underflow support, but leaving it because it might give an idea of my thought process on why): Not quite sure I fully followed this comment. I think my preference would be to error out for overflows/underflows, rather than silently allowing them. If things are going to be significantly more complex adding them but you are also going to address them immediately, I'm okay with it being deferred to a future patch. What I don't want long-term is for people to be able to write unintentionally broken test cases because they happen to be triggering underflow/overflow behaviour. Broken test cases are bad!

In D79936#2064448, @paulwalker-arm wrote:

I have a clang-format query. I'm getting failures because clang-format is suggesting to use "-<space><digit>" to format a negative number. This doesn't seem correct to me and is not the style I see for existing code in FileCheckTests.cpp. Is this something I can ignore?

@thopre ran into this recently too. I consider it a bug in clang-format personally, so you can ignore it, but if @thopre hasn't already, you should file a clang-format bug so that it can get fixed.

I'm planning to take a look at this clang-format bug today.

Seems to be related to the use of operator. I've created PR46157

[thopre]

In D79936#2066379, @thopre wrote:

In D79936#2066160, @thopre wrote:

I'm planning to take a look at this clang-format bug today.

Seems to be related to the use of operator. I've created PR46157

Someone already posted a patch for it: https://reviews.llvm.org/D80933. It works for 23ac16cf9bd4cc0bb434efcf6385baf083a2ff7b.

[paulwalker-arm]

llvm/lib/Support/FileCheck.cpp

486

For what it's worth I took a run at an implementation that doesn't require a call prefix and whilst almost as simple as suggested above there are a couple of downsides.

(1) Calls without a prefix require look ahead parsing, which means redundant work continually looking for functions that might never be there. For example, take the parsing of var_a + var_b + var_c + var_d - (var_e - var_f) where parseNumericOperand is likely to perform many failed parse attempts.

(2) Some diagnostics become harder or impossible. For example, is "mu(la+b)" a call to an unknown function, a missing operator or a bracket typo. I know the same scenario is true if the user forgets the prefix but when they don't, we can emit a more useful message. You can see another example in this patch where it's easier to spot and report a missing operator before a function call.

(3) A prefix allows the use of symbols that might otherwise be confusing. Tenuous I know but consider "a + !operator+(b+c)".

(4) Are there any plans for VAR1(VAR2+VAR3) as short hand for mul(VAR1, VAR2+VAR3)?

A downside of the prefix is that we cannot easily use "!" to mean "not". That said "!(var)" support is only a minor modification.

I don't know if these are strong reasons to go with a prefix and ultimately either approach works for me, so just let me know the preference and I'll make the necessary changes.

[jhenderson]

Taking conversation out-of-line to make it easier. My personal beliefs are as follows:

(1) Calls without a prefix require look ahead parsing, which means redundant work continually looking for functions that might never be there. For example, take the parsing of var_a + var_b + var_c + var_d - (var_e - var_f) where parseNumericOperand is likely to perform many failed parse attempts.

By look ahead parsing are you saying something like to identify whether var_a is actually a function, we have to parse the +? I've not given too much thought to this, but I think this extra work can be avoided by delaying handling of a token until the next token is identified. Thus an identifier token is left unprocessed until the next token has been read in, at which point it is either processed as a function or a variable. However, I accept that might need some rewriting of the existing parsing code. Trying to parse something as a numeric operand as a first attempt before trying to read it as something else seems like the wrong approach long-term as we add more power to these expressions.

(2) Some diagnostics become harder or impossible. For example, is "mu(la+b)" a call to an unknown function, a missing operator or a bracket typo. I know the same scenario is true if the user forgets the prefix but when they don't, we can emit a more useful message. You can see another example in this patch where it's easier to spot and report a missing operator before a function call.

I think it's okay in that case to treat that as an attempt to call function mu, which probably doesn't exist. I'm not sure having the prefix helps in this case: !mu(la+b) is just as unknown a function. The function name is delimited by the end of the previous token and the opening parenthesis in the unprefixed case. I'm not sure I see the case where it's easier to spot a missing operand. The examples in the patch are (I think):

• (1)(2) - this is unaffected - no identifier means these are treated as parenthesised expressions.
• 2(X) - 2 will be parsed as a numeric literal, so this is still a missing operator.
• 2!mul(FOO,2) - without the !, either the 2 is treated as a separate token, because it can't be the first character of an identifier (and therefore again a missing operator diagnostic is still possible), or 2mul becomes an invalid identifier, with corresponding message. I think either is an acceptable error.
• FOO !mul(FOO,2) - without the !, FOO is still a separate token because of the whitespace, so the missing operand is easily identifiable.

(3) A prefix allows the use of symbols that might otherwise be confusing. Tenuous I know but consider "a + !operator+(b+c)".

I think we either a) don't need to support such expressions or b) special-case + following the term operator or equivalent sequences. I think we just want to keep our function names to valid identifiers like we already have for variable names.

(4) Are there any plans for VAR1(VAR2+VAR3) as short hand for mul(VAR1, VAR2+VAR3)?

I'm not aware of any plans, and I don't think they're really needed (especially if we add support for * as a binary operator for multiplication).

A downside of the prefix is that we cannot easily use "!" to mean "not". That said "!(var)" support is only a minor modification.

I don't think we have any plans or need for boolean support (tools don't generally print "true" or "false" and other things like 1 or 0 can be supported using regular numeric expressions). We will probably want to support != as a comparator though at some point, so we need to allow for that.

I don't know if these are strong reasons to go with a prefix and ultimately either approach works for me, so just let me know the preference and I'll make the necessary changes.

My personal preference is still no prefix. Related aside is this comic: https://xkcd.com/1306/ (especially the alt text).

[paulwalker-arm]

Thanks for the discussion @jhenderson.

I've removed the function prefix and updated the tests accordingly. I've also removed div and mul support so that it's no longer dependent on D80915, which I'll progress separately.

[arichardson]

llvm/lib/Support/FileCheck.cpp

629

Should this be ltrim?

[paulwalker-arm]

llvm/lib/Support/FileCheck.cpp

629

It looks like redundant code as space is already trimmed before loop entry and exit. I'll remove it.

[paulwalker-arm]

Removed redundant rtrim.

[jhenderson]

A couple of test cases that might want adding:

1. Trying to use a variable as a function (e.g. VAR1(1, 2))
2. Trying to use a function as a variable (e.g. max + min)
3. Maybe even defining a variable explicitly as a recognised function name (e.g. max + max(1, 2) or even max + max(max, max)).

I reckon the first should be treated as an unrecognised variable, and the others allowed (although the second one probably would be using undefined variables).

llvm/docs/CommandGuide/FileCheck.rst

707

Perhaps "Accepted" rather than "Acceptable"

llvm/lib/Support/FileCheck.cpp

233

Should this be an Expected if it can't fail? Same for min.

255

This cantFail call suggests to me that this shouldn't be an Expected return.

374–380

We should probably allow for optional whitespace between the end of the function name and the (.

[paulwalker-arm]

llvm/lib/Support/FileCheck.cpp

233

This is to match the binop_eval_t typedef required by BinaryOperation.

255

As explained above. There did not seem much value in creating a duplicate set of functions (i.e. with and without an Expected result) given this is the only other use.

374–380

In LLVM's c/c++ world clang-format will remove such whitespace (presumably to aid readability) so do we really want to allow it in FileCheck code?

[jhenderson]

llvm/lib/Support/FileCheck.cpp

233

Thanks, makes sense.

374–380

We've allowed arbitrary whitespace everywhere else in the expressions, so I think we should. Not all environments will necessarily follow LLVM's coding standards.

[paulwalker-arm]

I initially misread the "max + max" related comments and went down the wrong path. Happily it was not in vain as it prompted me to reuse parseVariable because the function relates to identifiers rather than just variables. I've also simplied parseVariable a little but stopped short of renaming things.

[jhenderson]

Tip to help reviewers: click the "Done" box on inline comments before uploading a patch to indicate you have addressed a specific comment. The comment will then be marked as Done when you upload the next diff.

LGTM, thanks, but wait for others.

llvm/lib/Support/FileCheck.cpp

355

I know this was here before, but since you are modifying this line, you can fix on the way (or in a separate commit before if you prefer): use size_t rather than unsigned to match the return type of Str.size(). Optionally also update I to size_t for the same reason.

llvm/docs/CommandGuide/FileCheck.rst

* ``%<fmtspec>`` is the same matching format specifier as for defining numeric
   is used.  In case of conflict between matching formats of several numeric
   variables the format specifier is mandatory.
 
 * ``<expr>`` is an expression. An expression is in turn recursively defined
   as:
 
   * a numeric operand, or
   * an expression followed by an operator and a numeric operand.
 
-  A numeric operand is a previously defined numeric variable, or an integer
-  literal and have a 64-bit precision. The supported operators are ``+`` and
-  ``-``. Spaces are accepted before, after and between any of these elements.
-  Overflow and underflow are rejected. There is currently no support for
-  operator precendence, but parentheses can be used to change the evaluation
-  order.
+  A numeric operand is a previously defined numeric variable, an integer
+  literal, or a function. Spaces are accepted before, after and between any of
+  these elements. Numeric operands have 64-bit precision. Overflow and underflow
+  are rejected. There is no support for operator precendence, but parentheses
+  can be used to change the evaluation order.
+
+The supported operators are:
+
+  * ``+`` - Returns the sum of its two operands.
+  * ``-`` - Returns the difference of its two operands.
+
+The syntax of a function call is ``<name>(<arguments>)`` where:
+
+* ``name`` is a predefined string literal. Acceptable values are:
+
+  * add - Returns the sum of its two operands.
+  * max - Returns the largest of its two operands.
+  * min - Returns the smallest of its two operands.
+  * sub - Returns the difference of its two operands.
+
+* ``<arguments>`` is a comma seperated list of expressions.
 
 For example:
 
 .. code-block:: llvm
 
     ; CHECK: load r[[#REG:]], [r0]
     ; CHECK: load r[[#REG+1]], [r1]
     ; CHECK: Loading from 0x[[#%x,ADDR:]]
     ; CHECK-SAME: to 0x[[#ADDR + 7]]
 
 The above example would match the text:
 
 .. code-block:: gas
 
     load r5, [r0]
     load r6, [r1]
     Loading from 0xa0463440 to 0xa0463447
 
 but would not match the text:
 

llvm/lib/Support/FileCheck.cpp

if (AbsoluteDifference > MaxInt64) {
       Result -= static_cast<int64_t>(AbsoluteDifference);
       return ExpressionValue(Result);
     }
 
     return ExpressionValue(-static_cast<int64_t>(AbsoluteDifference));
   }
 }
 
+Expected<ExpressionValue> llvm::max(const ExpressionValue &LeftOperand,
+                                    const ExpressionValue &RightOperand) {
+  if (LeftOperand.isNegative() && RightOperand.isNegative()) {
+    int64_t LeftValue = cantFail(LeftOperand.getSignedValue());
+    int64_t RightValue = cantFail(RightOperand.getSignedValue());
+    return ExpressionValue(std::max(LeftValue, RightValue));
+  }
+
+  if (!LeftOperand.isNegative() && !RightOperand.isNegative()) {
+    uint64_t LeftValue = cantFail(LeftOperand.getUnsignedValue());
+    uint64_t RightValue = cantFail(RightOperand.getUnsignedValue());
+    return ExpressionValue(std::max(LeftValue, RightValue));
+  }
+
+  if (LeftOperand.isNegative())
+    return RightOperand;
+
+  return LeftOperand;
+}
+
+Expected<ExpressionValue> llvm::min(const ExpressionValue &LeftOperand,
+                                    const ExpressionValue &RightOperand) {
+  if (cantFail(max(LeftOperand, RightOperand)) == LeftOperand)
+    return RightOperand;
+
+  return LeftOperand;
+}
+
 Expected<ExpressionValue> NumericVariableUse::eval() const {
   Optional<ExpressionValue> Value = Variable->getValue();
   if (Value)
     return *Value;
 
   return make_error<UndefVarError>(getExpressionStr());
 }
 
BinaryOperation::getImplicitFormat(const SourceMgr &SM) const {
 
   return *LeftFormat != ExpressionFormat::Kind::NoFormat ? *LeftFormat
                                                          : *RightFormat;
 }
 
 Expected<std::string> NumericSubstitution::getResult() const {
   assert(ExpressionPointer->getAST() != nullptr &&
          "Substituting empty expression");
   Expected<ExpressionValue> EvaluatedValue =
       ExpressionPointer->getAST()->eval();
   if (!EvaluatedValue)
     return EvaluatedValue.takeError();
   ExpressionFormat Format = ExpressionPointer->getFormat();
   return Format.getMatchingString(*EvaluatedValue);
 }
 
 Expected<std::string> StringSubstitution::getResult() const {
   // Look up the value and escape it so that we can put it into the regex.
   Expected<StringRef> VarVal = Context->getPatternVarValue(FromStr);
   if (!VarVal)
     return VarVal.takeError();
   return Regex::escape(*VarVal);
 }
 
+// Variable names are composed of a leading alpha character or underscore
+// followed by a sequence of alphanumeric characters and/or underscores.
 bool Pattern::isValidVarNameStart(char C) { return C == '_' || isAlpha(C); }
+bool Pattern::isValidVarNameChar(char C) { return C == '_' || isAlnum(C); }
 
 Expected<Pattern::VariableProperties>
 Pattern::parseVariable(StringRef &Str, const SourceMgr &SM) {
   if (Str.empty())
     return ErrorDiagnostic::get(SM, Str, "empty variable name");
 
   bool ParsedOneChar = false;
   unsigned I = 0;
   bool IsPseudo = Str[0] == '@';
 
   // Global vars start with '$'.
   if (Str[0] == '$' || IsPseudo)
     ++I;
 
   for (unsigned E = Str.size(); I != E; ++I) {
     if (!ParsedOneChar && !isValidVarNameStart(Str[I]))
       return ErrorDiagnostic::get(SM, Str, "invalid variable name");
 
-    // Variable names are composed of alphanumeric characters and underscores.
-    if (Str[I] != '_' && !isAlnum(Str[I]))
+    if (!isValidVarNameChar(Str[I]))
       break;
     ParsedOneChar = true;
   }
 
   StringRef Name = Str.take_front(I);
   Str = Str.substr(I);
   return VariableProperties {Name, IsPseudo};
 }
 
+// Parsing helper function that returns true if Str starts with what looks like
+// a function call. Function names have the same format as variable names.
+bool Pattern::startsWithCallExpr(StringRef &Str) {
+  if (Str.empty())
+    return false;
+
+  if (!isValidVarNameStart(Str[0]))
+    return false;
+
+  for (unsigned I = 1, E = Str.size(); I != E; ++I) {
+    // Have we reached the end of the function name of a call expression?
+    if (Str[I] == '(')
+      return true;
+    if (!isValidVarNameChar(Str[I]))
+      break;
+  }
+
+  return false;
+}
+
 // StringRef holding all characters considered as horizontal whitespaces by
 // FileCheck input canonicalization.
 constexpr StringLiteral SpaceChars = " \t";
 
 // Parsing helper function that strips the first character in S and returns it.
 static char popFront(StringRef &S) {
   char C = S.front();
   S = S.drop_front();
Expected<std::unique_ptr<ExpressionAST>> Pattern::parseNumericOperand(
     FileCheckPatternContext *Context, const SourceMgr &SM) {
   if (Expr.startswith("(")) {
     if (AO != AllowedOperand::Any)
       return ErrorDiagnostic::get(
           SM, Expr, "parenthesized expression not permitted here");
     return parseParenExpr(Expr, LineNumber, Context, SM);
   }
 
+  // Try to parse a function call.
+  if (startsWithCallExpr(Expr)) {
+    if (AO != AllowedOperand::Any)
+      return ErrorDiagnostic::get(SM, Expr, "unexpected function call");
+
+    return parseCallExpr(Expr, LineNumber, Context, SM);
+  }
+
   if (AO == AllowedOperand::LineVar || AO == AllowedOperand::Any) {
     // Try to parse as a numeric variable use.
     Expected<Pattern::VariableProperties> ParseVarResult =
         parseVariable(Expr, SM);
     if (ParseVarResult)
       return parseNumericVariableUse(ParseVarResult->Name,
                                      ParseVarResult->IsPseudo, LineNumber,
                                      Context, SM);
Pattern::parseBinop(StringRef Expr, StringRef &RemainingExpr,
   if (!RightOpResult)
     return RightOpResult;
 
   Expr = Expr.drop_back(RemainingExpr.size());
   return std::make_unique<BinaryOperation>(Expr, EvalBinop, std::move(LeftOp),
                                            std::move(*RightOpResult));
 }
 
+Expected<std::unique_ptr<ExpressionAST>>
+Pattern::parseCallExpr(StringRef &Expr, Optional<size_t> LineNumber,
+                       FileCheckPatternContext *Context, const SourceMgr &SM) {
+  assert(startsWithCallExpr(Expr) && "Expected call expression!");
+  size_t FuncNameEnd = Expr.find('(');
+
+  StringRef FuncName = Expr.take_front(FuncNameEnd);
+  auto OptFunc = StringSwitch<Optional<binop_eval_t>>(FuncName)
+                     .Case("add", operator+)
+                     .Case("max", max)
+                     .Case("min", min)
+                     .Case("sub", operator-)
+                     .Default(None);
+
+  if (!OptFunc)
+    return ErrorDiagnostic::get(
+        SM, Expr, Twine("call to undefined function '") + FuncName + "'");
+
+  // Record this for diagnostics that should be tied to the function name.
+  SMLoc OpLoc = SMLoc::getFromPointer(Expr.data());
+
+  // Consume function name along with leading '(';
+  Expr = Expr.drop_front(FuncNameEnd + 1);
+  Expr = Expr.ltrim(SpaceChars);
+
+  // Parse call arguments, which are comma separated.
+  SmallVector<std::unique_ptr<ExpressionAST>, 4> Args;
+  while (!Expr.empty() && !Expr.startswith(")")) {
+    Expr = Expr.rtrim(SpaceChars);
+    if (Expr.startswith(","))
+      return ErrorDiagnostic::get(SM, Expr, "missing argument");
+
+    // Parse the argument, which is an arbitary expression.
+    StringRef OuterBinOpExpr = Expr;
+    Expected<std::unique_ptr<ExpressionAST>> Arg =
+        parseNumericOperand(Expr, AllowedOperand::Any, LineNumber, Context, SM);
+    while (Arg && !Expr.empty()) {
+      Expr = Expr.ltrim(SpaceChars);
+      // Have we reached an argument terminator?
+      if (Expr.startswith(",") || Expr.startswith(")"))
+        break;
+
+      // Arg = Arg <op> <expr>
+      Arg = parseBinop(OuterBinOpExpr, Expr, std::move(*Arg), false, LineNumber,
+                       Context, SM);
+    }
+
+    // Prefer an expression error over a generic invalid argument message.
+    if (!Arg)
+      return Arg.takeError();
+    Args.push_back(std::move(*Arg));
+
+    // Have we parsed all available arguments?
+    Expr = Expr.ltrim(SpaceChars);
+    if (!Expr.consume_front(","))
+      break;
+
+    Expr = Expr.ltrim(SpaceChars);
+    if (Expr.startswith(")"))
+      return ErrorDiagnostic::get(SM, Expr, "missing argument");
+  }
+
+  if (!Expr.consume_front(")"))
+    return ErrorDiagnostic::get(SM, Expr,
+                                "missing ')' at end of call expression");
+
+  const unsigned NumArgs = Args.size();
+  if (NumArgs == 2)
+    return std::make_unique<BinaryOperation>(Expr, *OptFunc, std::move(Args[0]),
+                                             std::move(Args[1]));
+
+  // TODO: Support more than binop_eval_t.
+  return ErrorDiagnostic::get(SM, OpLoc,
+                              Twine("function '") + FuncName +
+                                  Twine("' takes 2 arguments but ") +
+                                  Twine(NumArgs) + " given");
+}
+
 Expected<std::unique_ptr<Expression>> Pattern::parseNumericSubstitutionBlock(
     StringRef Expr, Optional<NumericVariable *> &DefinedNumericVariable,
     bool IsLegacyLineExpr, Optional<size_t> LineNumber,
     FileCheckPatternContext *Context, const SourceMgr &SM) {
   std::unique_ptr<ExpressionAST> ExpressionASTPointer = nullptr;
   StringRef DefExpr = StringRef();
   DefinedNumericVariable = None;
   ExpressionFormat ExplicitFormat = ExpressionFormat();
 
-  // Parse format specifier.
+  // Parse format specifier (NOTE: ',' is also an argument seperator).
   size_t FormatSpecEnd = Expr.find(',');
-  if (FormatSpecEnd != StringRef::npos) {
+  size_t FunctionStart = Expr.find('(');
+  if (FormatSpecEnd != StringRef::npos && FormatSpecEnd < FunctionStart) {
     Expr = Expr.ltrim(SpaceChars);
     if (!Expr.consume_front("%"))
       return ErrorDiagnostic::get(
           SM, Expr, "invalid matching format specification in expression");
 
     // Check for unknown matching format specifier and set matching format in
     // class instance representing this expression.
     SMLoc fmtloc = SMLoc::getFromPointer(Expr.data());
     switch (popFront(Expr)) {
     case 'u':

llvm/lib/Support/FileCheckImpl.h

 };
 
 /// Performs operation and \returns its result or an error in case of failure,
 /// such as if an overflow occurs.
 Expected<ExpressionValue> operator+(const ExpressionValue &Lhs,
                                     const ExpressionValue &Rhs);
 Expected<ExpressionValue> operator-(const ExpressionValue &Lhs,
                                     const ExpressionValue &Rhs);
+Expected<ExpressionValue> max(const ExpressionValue &Lhs,
+                              const ExpressionValue &Rhs);
+Expected<ExpressionValue> min(const ExpressionValue &Lhs,
+                              const ExpressionValue &Rhs);
 
 /// Base class representing the AST of a given expression.
 class ExpressionAST {
 private:
   StringRef ExpressionStr;
 
 public:
   ExpressionAST(StringRef ExpressionStr) : ExpressionStr(ExpressionStr) {}
public:
 
   /// \returns the pointer to the global state for all patterns in this
   /// FileCheck instance.
   FileCheckPatternContext *getContext() const { return Context; }
 
   /// \returns whether \p C is a valid first character for a variable name.
   static bool isValidVarNameStart(char C);
 
+  /// \returns whether \p C is a valid non-first character for a variable name.
+  static bool isValidVarNameChar(char C);
+
   /// Parsing information about a variable.
   struct VariableProperties {
     StringRef Name;
     bool IsPseudo;
   };
 
   /// Parses the string at the start of \p Str for a variable name. \returns
   /// a VariableProperties structure holding the variable name and whether it
private:
   /// Finds the closing sequence of a regex variable usage or definition.
   ///
   /// \p Str has to point in the beginning of the definition (right after the
   /// opening sequence). \p SM holds the SourceMgr used for error reporting.
   ///  \returns the offset of the closing sequence within Str, or npos if it
   /// was not found.
   static size_t FindRegexVarEnd(StringRef Str, SourceMgr &SM);
 
+  /// Returns whether \p Str starts with a valid function name followed by an
+  /// opening parenthesis.
+  static bool startsWithCallExpr(StringRef &Str);
+
   /// Parses \p Expr for the name of a numeric variable to be defined at line
   /// \p LineNumber, or before input is parsed if \p LineNumber is None.
   /// \returns a pointer to the class instance representing that variable,
   /// creating it if needed, or an error holding a diagnostic against \p SM
   /// should defining such a variable be invalid.
   static Expected<NumericVariable *> parseNumericVariableDefinition(
       StringRef &Expr, FileCheckPatternContext *Context,
       Optional<size_t> LineNumber, ExpressionFormat ImplicitFormat,
       const SourceMgr &SM);
   /// Parses \p Name as a (pseudo if \p IsPseudo is true) numeric variable use
   /// at line \p LineNumber, or before input is parsed if \p LineNumber is
   /// None. Parameter \p Context points to the class instance holding the live
   /// string and numeric variables. \returns the pointer to the class instance
   /// representing that variable if successful, or an error holding a
   /// diagnostic against \p SM otherwise.
   static Expected<std::unique_ptr<NumericVariableUse>> parseNumericVariableUse(
       StringRef Name, bool IsPseudo, Optional<size_t> LineNumber,
       FileCheckPatternContext *Context, const SourceMgr &SM);
   enum class AllowedOperand { LineVar, LegacyLiteral, Any };
   /// Parses \p Expr for use of a numeric operand at line \p LineNumber, or
-  /// before input is parsed if \p LineNumber is None. Accepts both literal
-  /// values and numeric variables, depending on the value of \p AO. Parameter
-  /// \p Context points to the class instance holding the live string and
-  /// numeric variables. \returns the class representing that operand in the
+  /// before input is parsed if \p LineNumber is None. Accepts literal values,
+  /// numeric variables and function calls, depending on the value of \p AO.
+  /// Parameter \p Context points to the class instance holding the live string
+  /// and numeric variables. \returns the class representing that operand in the
   /// AST of the expression or an error holding a diagnostic against \p SM
   /// otherwise. If \p Expr starts with a "(" this function will attempt to
   /// parse a parenthesized expression.
   static Expected<std::unique_ptr<ExpressionAST>>
   parseNumericOperand(StringRef &Expr, AllowedOperand AO,
                       Optional<size_t> LineNumber,
                       FileCheckPatternContext *Context, const SourceMgr &SM);
   /// Parses and updates \p RemainingExpr for a binary operation at line
   /// \p LineNumber, or before input is parsed if \p LineNumber is None. The
   /// left operand of this binary operation is given in \p LeftOp and \p Expr
private:
   /// before input is parsed if \p LineNumber is None. \p Expr must start with
   /// a '('. Accepts both literal values and numeric variables. Parameter \p
   /// Context points to the class instance holding the live string and numeric
   /// variables. \returns the class representing that operand in the AST of the
   /// expression or an error holding a diagnostic against \p SM otherwise.
   static Expected<std::unique_ptr<ExpressionAST>>
   parseParenExpr(StringRef &Expr, Optional<size_t> LineNumber,
                  FileCheckPatternContext *Context, const SourceMgr &SM);
+
+  /// Parses \p Expr for a function call at line \p LineNumber, or before input
+  /// is parsed if \p LineNumber is None. Parameter \p Context points to the
+  /// class instance holding the live string and numeric variables. \returns the
+  /// class representing that call in the AST of the expression or an error
+  /// holding a diagnostic against \p SM otherwise.
+  static Expected<std::unique_ptr<ExpressionAST>>
+  parseCallExpr(StringRef &Expr, Optional<size_t> LineNumber,
+                FileCheckPatternContext *Context, const SourceMgr &SM);
 };
 
 //===----------------------------------------------------------------------===//
 // Check Strings.
 //===----------------------------------------------------------------------===//
 
 /// A check that we found in the input file.
 struct FileCheckString {

llvm/test/FileCheck/numeric-expression.txt

 INVALID-FMT-SPEC-MSG2-NEXT:    {{^}}                                    ^{{$}}
 
 ; Numeric expressions in explicit matching format and default matching rule using
 ; variables defined on other lines without spaces.
 USE EXPL FMT IMPL MATCH  // CHECK-LABEL: USE EXPL FMT IMPL MATCH
 11  // CHECK-NEXT: {{^}}[[#%u,UNSI]]
 12  // CHECK-NEXT: {{^}}[[#%u,UNSI+1]]
 10  // CHECK-NEXT: {{^}}[[#%u,UNSI-1]]
+15  // CHECK-NEXT: {{^}}[[#%u,add(UNSI,4)]]
+11  // CHECK-NEXT: {{^}}[[#%u,max(UNSI,7)]]
+99  // CHECK-NEXT: {{^}}[[#%u,max(UNSI,99)]]
+7   // CHECK-NEXT: {{^}}[[#%u,min(UNSI,7)]]
+11  // CHECK-NEXT: {{^}}[[#%u,min(UNSI,99)]]
+8   // CHECK-NEXT: {{^}}[[#%u,sub(UNSI,3)]]
 c   // CHECK-NEXT: {{^}}[[#%x,LHEX]]
 d   // CHECK-NEXT: {{^}}[[#%x,LHEX+1]]
 b   // CHECK-NEXT: {{^}}[[#%x,LHEX-1]]
 1a  // CHECK-NEXT: {{^}}[[#%x,LHEX+0xe]]
 1a  // CHECK-NEXT: {{^}}[[#%x,LHEX+0xE]]
+e   // CHECK-NEXT: {{^}}[[#%x,add(LHEX,2)]]
+ff  // CHECK-NEXT: {{^}}[[#%x,max(LHEX,0xff)]]
+a   // CHECK-NEXT: {{^}}[[#%x,min(LHEX,0xa)]]
+a   // CHECK-NEXT: {{^}}[[#%x,sub(LHEX,2)]]
 D   // CHECK-NEXT: {{^}}[[#%X,UHEX]]
 E   // CHECK-NEXT: {{^}}[[#%X,UHEX+1]]
 C   // CHECK-NEXT: {{^}}[[#%X,UHEX-1]]
 1B  // CHECK-NEXT: {{^}}[[#%X,UHEX+0xe]]
 1B  // CHECK-NEXT: {{^}}[[#%X,UHEX+0xE]]
+F   // CHECK-NEXT: {{^}}[[#%X,add(UHEX,2)]]
+FF  // CHECK-NEXT: {{^}}[[#%X,max(UHEX,0xff)]]
+A   // CHECK-NEXT: {{^}}[[#%X,min(UHEX,0xa)]]
+B   // CHECK-NEXT: {{^}}[[#%X,sub(UHEX,2)]]
 -30 // CHECK-NEXT: {{^}}[[#%d,SIGN]]
 -29 // CHECK-NEXT: {{^}}[[#%d,SIGN+1]]
 -31 // CHECK-NEXT: {{^}}[[#%d,SIGN-1]]
 42  // CHECK-NEXT: {{^}}[[#%d,SIGN+72]]
+-29 // CHECK-NEXT: {{^}}[[#%d,add(SIGN,1)]]
+-17 // CHECK-NEXT: {{^}}[[#%d,max(SIGN,-17)]]
+-30 // CHECK-NEXT: {{^}}[[#%d,min(SIGN,-17)]]
+-31 // CHECK-NEXT: {{^}}[[#%d,sub(SIGN,1)]]
 11  // CHECK-NEXT: {{^}}[[#%u,UNSIa]]
 11  // CHECK-NEXT: {{^}}[[#%u,UNSIb]]
 11  // CHECK-NEXT: {{^}}[[#%u,UNSIc]]
 c   // CHECK-NEXT: {{^}}[[#%x,LHEXa]]
 
 ; Numeric expressions in explicit matching format and default matching rule using
 ; variables defined on other lines with different spacing.
 USE EXPL FMT IMPL MATCH SPC  // CHECK-LABEL: USE EXPL FMT IMPL MATCH SPC
 11  // CHECK-NEXT: {{^}}[[#%u, UNSI]]
 11  // CHECK-NEXT: {{^}}[[# %u, UNSI]]
 11  // CHECK-NEXT: {{^}}[[# %u, UNSI ]]
 12  // CHECK-NEXT: {{^}}[[#%u, UNSI+1]]
 12  // CHECK-NEXT: {{^}}[[# %u, UNSI+1]]
 12  // CHECK-NEXT: {{^}}[[# %u , UNSI+1]]
 12  // CHECK-NEXT: {{^}}[[# %u , UNSI +1]]
 12  // CHECK-NEXT: {{^}}[[# %u , UNSI + 1]]
 12  // CHECK-NEXT: {{^}}[[# %u , UNSI + 1 ]]
 10  // CHECK-NEXT: {{^}}[[#%u, UNSI-1]]
 10  // CHECK-NEXT: {{^}}[[# %u, UNSI-1]]
 10  // CHECK-NEXT: {{^}}[[# %u , UNSI-1]]
 10  // CHECK-NEXT: {{^}}[[# %u , UNSI -1]]
 10  // CHECK-NEXT: {{^}}[[# %u , UNSI - 1]]
 10  // CHECK-NEXT: {{^}}[[# %u , UNSI - 1 ]]
+13  // CHECK-NEXT: {{^}}[[#%u, add(UNSI,2)]]
+13  // CHECK-NEXT: {{^}}[[# %u, add(UNSI,2)]]
+13  // CHECK-NEXT: {{^}}[[# %u , add(UNSI,2)]]
+13  // CHECK-NEXT: {{^}}[[# %u , add(UNSI, 2)]]
+13  // CHECK-NEXT: {{^}}[[# %u , add( UNSI, 2)]]
+13  // CHECK-NEXT: {{^}}[[# %u , add( UNSI,2)]]
+13  // CHECK-NEXT: {{^}}[[# %u , add(UNSI,2) ]]
+104 // CHECK-NEXT: {{^}}[[# %u , UNSI + sub( add(100 , UNSI+ 1 ), 20) +1 ]]
 
 ; Numeric expressions in implicit matching format and default matching rule using
 ; variables defined on other lines.
 USE IMPL FMT IMPL MATCH  // CHECK-LABEL: USE IMPL FMT IMPL MATCH
 11  // CHECK-NEXT: {{^}}[[#UNSI]]
 12  // CHECK-NEXT: {{^}}[[#UNSI+1]]
 10  // CHECK-NEXT: {{^}}[[#UNSI-1]]
+99  // CHECK-NEXT: {{^}}[[#max(UNSI,99)]]
+7   // CHECK-NEXT: {{^}}[[#min(UNSI,7)]]
 c   // CHECK-NEXT: {{^}}[[#LHEX]]
 d   // CHECK-NEXT: {{^}}[[#LHEX+1]]
 b   // CHECK-NEXT: {{^}}[[#LHEX-1]]
 1a  // CHECK-NEXT: {{^}}[[#LHEX+0xe]]
 1a  // CHECK-NEXT: {{^}}[[#LHEX+0xE]]
+ff  // CHECK-NEXT: {{^}}[[#max(LHEX,255)]]
+a   // CHECK-NEXT: {{^}}[[#min(LHEX,10)]]
 D   // CHECK-NEXT: {{^}}[[#UHEX]]
 E   // CHECK-NEXT: {{^}}[[#UHEX+1]]
 C   // CHECK-NEXT: {{^}}[[#UHEX-1]]
 1B  // CHECK-NEXT: {{^}}[[#UHEX+0xe]]
 1B  // CHECK-NEXT: {{^}}[[#UHEX+0xE]]
+FF  // CHECK-NEXT: {{^}}[[#max(UHEX,255)]]
+A   // CHECK-NEXT: {{^}}[[#min(UHEX,10)]]
 -30 // CHECK-NEXT: {{^}}[[#SIGN]]
 -29 // CHECK-NEXT: {{^}}[[#SIGN+1]]
 -31 // CHECK-NEXT: {{^}}[[#SIGN-1]]
 
 ; Numeric expressions using variables defined on other lines and an immediate
 ; interpreted as an unsigned value.
 ; Note: 9223372036854775819 = 0x8000000000000000 + 11
 USE IMPL FMT IMPL MATCH UNSIGNED IMM

 REDEF-NEW-FMT-MSG-NEXT: {{R}}EDEF-NEW-FMT-NEXT: {{\[\[#%X,UNSI:\]\]}}
 REDEF-NEW-FMT-MSG-NEXT:    {{^}}                              ^{{$}}
 
 ; Numeric expression with overflow.
 RUN: not FileCheck --check-prefix OVERFLOW --input-file %s %s 2>&1 \
 RUN:   | FileCheck --check-prefix OVERFLOW-MSG --strict-whitespace %s
 
 OVERFLOW
 BIGVAR=10000000000000000
 OVERFLOW-LABEL: OVERFLOW
 OVERFLOW-NEXT: BIGVAR: [[#BIGVAR:0x8000000000000000+0x8000000000000000]]
 OVERFLOW-MSG: numeric-expression.txt:[[#@LINE-1]]:27: error: unable to substitute variable or numeric expression
 OVERFLOW-MSG-NEXT: {{O}}VERFLOW-NEXT: BIGVAR: {{\[\[#BIGVAR:0x8000000000000000\+0x8000000000000000\]\]}}
 OVERFLOW-MSG-NEXT:    {{^}}                          ^{{$}}
 
 ; Numeric expression with underflow.
 RUN: not FileCheck --check-prefix UNDERFLOW --input-file %s %s 2>&1 \
 RUN:   | FileCheck --check-prefix UNDERFLOW-MSG --strict-whitespace %s
 
 UNDERFLOW
 TINYVAR=-10000000000000000
 UNDERFLOW-LABEL: UNDERFLOW
 UNDERFLOW-NEXT: TINYVAR: [[#%d,TINYVAR:-0x8000000000000000-0x8000000000000000]]
 UNDERFLOW-MSG: numeric-expression.txt:[[#@LINE-1]]:29: error: unable to substitute variable or numeric expression
 UNDERFLOW-MSG-NEXT: {{U}}NDERFLOW-NEXT: TINYVAR: {{\[\[#%d,TINYVAR:-0x8000000000000000-0x8000000000000000\]\]}}
 UNDERFLOW-MSG-NEXT:    {{^}}                            ^{{$}}
+
+RUN: %ProtectFileCheckOutput \
+RUN: not FileCheck -D#NUMVAR=10 --check-prefix CALL-MISSING-CLOSING-BRACKET --input-file %s %s 2>&1 \
+RUN:   | FileCheck --strict-whitespace --check-prefix CALL-MISSING-CLOSING-BRACKET-MSG %s
+
+CALL MISSING CLOSING BRACKET
+30
+CALL-MISSING-CLOSING-BRACKET-LABEL: CALL MISSING CLOSING BRACKET
+CALL-MISSING-CLOSING-BRACKET-NEXT: [[#add(NUMVAR,3]]
+CALL-MISSING-CLOSING-BRACKET-MSG: numeric-expression.txt:[[#@LINE-1]]:51: error: missing ')' at end of call expression
+CALL-MISSING-CLOSING-BRACKET-MSG-NEXT: {{C}}ALL-MISSING-CLOSING-BRACKET-NEXT: {{\[\[#add\(NUMVAR,3\]\]}}
+CALL-MISSING-CLOSING-BRACKET-MSG-NEXT:      {{^}}                                                  ^{{$}}
+
+RUN: %ProtectFileCheckOutput \
+RUN: not FileCheck -D#NUMVAR=10 --check-prefix CALL-MISSING-ARGUMENT --input-file %s %s 2>&1 \
+RUN:   | FileCheck --strict-whitespace --check-prefix CALL-MISSING-ARGUMENT-MSG %s
+
+CALL MISSING ARGUMENT
+30
+CALL-MISSING-ARGUMENT-LABEL: CALL MISSING ARGUMENT
+CALL-MISSING-ARGUMENT-NEXT: [[#add(NUMVAR,)]]
+CALL-MISSING-ARGUMENT-MSG: numeric-expression.txt:[[#@LINE-1]]:43: error: missing argument
+CALL-MISSING-ARGUMENT-MSG-NEXT: {{C}}ALL-MISSING-ARGUMENT-NEXT: {{\[\[#add\(NUMVAR,\)\]\]}}
+CALL-MISSING-ARGUMENT-MSG-NEXT:      {{^}}                                          ^{{$}}
+
+RUN: %ProtectFileCheckOutput \
+RUN: not FileCheck -D#NUMVAR=10 --check-prefix CALL-WRONG-ARGUMENT-COUNT --input-file %s %s 2>&1 \
+RUN:   | FileCheck --strict-whitespace --check-prefix CALL-WRONG-ARGUMENT-COUNT-MSG %s
+
+CALL WRONG ARGUMENT COUNT
+30
+CALL-WRONG-ARGUMENT-COUNT-LABEL: CALL WRONG ARGUMENT COUNT
+CALL-WRONG-ARGUMENT-COUNT-NEXT: [[#add(NUMVAR)]]
+CALL-WRONG-ARGUMENT-COUNT-MSG: numeric-expression.txt:[[#@LINE-1]]:36: error: function 'add' takes 2 arguments but 1 given
+CALL-WRONG-ARGUMENT-COUNT-MSG-NEXT: {{C}}ALL-WRONG-ARGUMENT-COUNT-NEXT: {{\[\[#add\(NUMVAR\)\]\]}}
+CALL-WRONG-ARGUMENT-COUNT-MSG-NEXT:    {{^}}                                   ^{{$}}
+
+RUN: %ProtectFileCheckOutput \
+RUN: not FileCheck -D#NUMVAR=10 --check-prefix CALL-UNDEFINED-FUNCTION --input-file %s %s 2>&1 \
+RUN:   | FileCheck --strict-whitespace --check-prefix CALL-UNDEFINED-FUNCTION-MSG %s
+
+CALL UNDEFINED FUNCTION
+30
+CALL-UNDEFINED-FUNCTION-LABEL: CALL UNDEFINED FUNCTION
+CALL-UNDEFINED-FUNCTION-NEXT: [[#bogus_function(NUMVAR)]]
+CALL-UNDEFINED-FUNCTION-MSG: numeric-expression.txt:[[#@LINE-1]]:34: error: call to undefined function 'bogus_function'
+CALL-UNDEFINED-FUNCTION-MSG-NEXT: {{C}}ALL-UNDEFINED-FUNCTION-NEXT: {{\[\[#bogus_function\(NUMVAR\)\]\]}}
+CALL-UNDEFINED-FUNCTION-MSG-NEXT:    {{^}}                                 ^{{$}}

llvm/unittests/Support/FileCheckTest.cpp

TEST_F(FileCheckTest, ParseNumericSubstitutionBlock) {
   expectDiagnosticError("unsupported operation ')'",
                         Tester.parseSubst("1)").takeError());
   expectDiagnosticError("unsupported operation ')'",
                         Tester.parseSubst("(1+2))").takeError());
   expectDiagnosticError("unsupported operation ')'",
                         Tester.parseSubst("(2))").takeError());
   expectDiagnosticError("unsupported operation ')'",
                         Tester.parseSubst("(1))(").takeError());
+
+  // Valid expression with function call.
+  EXPECT_THAT_EXPECTED(Tester.parseSubst("add(FOO,3)"), Succeeded());
+  // Valid expression with nested function call.
+  EXPECT_THAT_EXPECTED(Tester.parseSubst("add(FOO, min(BAR,10))"), Succeeded());
+  // Valid expression with function call taking expression as argument.
+  EXPECT_THAT_EXPECTED(Tester.parseSubst("add(FOO, (BAR+10) + 3)"),
+                       Succeeded());
+  EXPECT_THAT_EXPECTED(Tester.parseSubst("add(FOO, min(BAR,10) + 3)"),
+                       Succeeded());
+
+  // Malformed call syntax.
+  expectDiagnosticError("missing ')' at end of call expression",
+                        Tester.parseSubst("add(FOO,(BAR+7)").takeError());
+  expectDiagnosticError("missing ')' at end of call expression",
+                        Tester.parseSubst("add(FOO,min(BAR,7)").takeError());
+  expectDiagnosticError("missing argument",
+                        Tester.parseSubst("add(FOO,)").takeError());
+  expectDiagnosticError("missing argument",
+                        Tester.parseSubst("add(,FOO)").takeError());
+  expectDiagnosticError("missing argument",
+                        Tester.parseSubst("add(FOO,,3)").takeError());
+
+  // Valid call, but to an unknown function.
+  expectDiagnosticError("call to undefined function 'bogus_function'",
+                        Tester.parseSubst("bogus_function(FOO,3)").takeError());
+
+  // Valid call, but with incorrect argument count.
+  expectDiagnosticError("function 'add' takes 2 arguments but 1 given",
+                        Tester.parseSubst("add(FOO)").takeError());
+  expectDiagnosticError("function 'add' takes 2 arguments but 3 given",
+                        Tester.parseSubst("add(FOO,3,4)").takeError());
+
+  // Valid call, but not part of a valid expression.
+  expectDiagnosticError("unsupported operation 'a'",
+                        Tester.parseSubst("2add(FOO,2)").takeError());
+  expectDiagnosticError("unsupported operation 'a'",
+                        Tester.parseSubst("FOO add(FOO,2)").takeError());
+  expectDiagnosticError("unsupported operation 'a'",
+                        Tester.parseSubst("add(FOO,2)add(FOO,2)").takeError());
 }
 
 TEST_F(FileCheckTest, ParsePattern) {
   PatternTester Tester;
 
   // Invalid space in string substitution.
   EXPECT_TRUE(Tester.parsePattern("[[ BAR]]"));
 
TEST_F(FileCheckTest, MatchParen) {
   Tester.initNextPattern();
   ASSERT_FALSE(Tester.parsePattern("[[#(NUMVAR)]]"));
   EXPECT_THAT_EXPECTED(Tester.match("18"), Succeeded());
   Tester.initNextPattern();
   ASSERT_FALSE(Tester.parsePattern("[[#(NUMVAR+2)]]"));
   EXPECT_THAT_EXPECTED(Tester.match("20"), Succeeded());
 }
 
+TEST_F(FileCheckTest, MatchBuiltinFunctions) {
+  PatternTester Tester;
+  // Esnure #NUMVAR has the expected value.
+  Tester.initNextPattern();
+  ASSERT_FALSE(Tester.parsePattern("[[#NUMVAR:]]"));
+  expectNotFoundError(Tester.match("FAIL").takeError());
+  expectNotFoundError(Tester.match("").takeError());
+  EXPECT_THAT_EXPECTED(Tester.match("18"), Succeeded());
+
+  // Check each builtin function generates the expected result.
+  Tester.initNextPattern();
+  ASSERT_FALSE(Tester.parsePattern("[[#add(NUMVAR,13)]]"));
+  EXPECT_THAT_EXPECTED(Tester.match("31"), Succeeded());
+  Tester.initNextPattern();
+  ASSERT_FALSE(Tester.parsePattern("[[#sub(NUMVAR,7)]]"));
+  EXPECT_THAT_EXPECTED(Tester.match("11"), Succeeded());
+  Tester.initNextPattern();
+  ASSERT_FALSE(Tester.parsePattern("[[#max(NUMVAR,5)]]"));
+  EXPECT_THAT_EXPECTED(Tester.match("18"), Succeeded());
+  Tester.initNextPattern();
+  ASSERT_FALSE(Tester.parsePattern("[[#max(NUMVAR,99)]]"));
+  EXPECT_THAT_EXPECTED(Tester.match("99"), Succeeded());
+  Tester.initNextPattern();
+  ASSERT_FALSE(Tester.parsePattern("[[#min(NUMVAR,5)]]"));
+  EXPECT_THAT_EXPECTED(Tester.match("5"), Succeeded());
+  Tester.initNextPattern();
+  ASSERT_FALSE(Tester.parsePattern("[[#min(NUMVAR,99)]]"));
+  EXPECT_THAT_EXPECTED(Tester.match("18"), Succeeded());
+
+  // Check nested function calls.
+  Tester.initNextPattern();
+  ASSERT_FALSE(Tester.parsePattern("[[#add(min(7,2),max(4,10))]]"));
+  EXPECT_THAT_EXPECTED(Tester.match("12"), Succeeded());
+}
+
 TEST_F(FileCheckTest, Substitution) {
   SourceMgr SM;
   FileCheckPatternContext Context;
   EXPECT_THAT_ERROR(Context.defineCmdlineVariables({"FOO=BAR"}, SM),
                     Succeeded());
 
   // Substitution of an undefined string variable fails and error holds that
   // variable's name.