FileCheck [10/12]: Add support for signed numeric values

[thopre]

Summary

This patch is part of a patch series to add support for FileCheck
numeric expressions. This specific patch adds support signed numeric
values, thus allowing negative numeric values.

As such, the patch adds a new class to represent a signed or unsigned
value and add the logic for type promotion and type conversion in
numeric expression mixing signed and unsigned values. It also adds
the %d format specifier to represent signed value.

Finally, it also adds underflow and overflow detection when performing a
binary operation.

Copyright:

• Linaro (changes up to diff 183612 of revision D55940)
• GraphCore (changes in later versions of revision D55940 and in new revision created off D55940)

[thopre]

Rebase on top of latest changes in the series

[thopre]

Rebase on top of latest changes in the patch series

[thopre]

• Fix a case of numeric expression -> expression
• Fix code style

[thopre]

Rebase on latest changes in patch series

[arichardson]

llvm/lib/Support/FileCheck.cpp

91

llvm/include/llvm/Support/CheckedArithmetic.h contains llvm::checkedAdd, llvm::checkedSub, llvm::checkedAddUnsigned and llvm::checkedSubUnsigned, which should make this code much simpler.

[thopre]

Address comment and rebase

[thopre]

Uploading the current status of this patch. I know there are still issues (in particular add more unittests), and I'll address them.

[thopre]

Rebase

[thopre]

Rebase

[thopre]

Rebase

[thopre]

Need to add unit tests

[arichardson]

Looks good to me once the tests have been added and other reviewers are also happy.

llvm/lib/Support/FileCheck.cpp

84

This is fine for the initial version of the patch, but I think in the future it will be useful to permit adding an unsigned variable to a signed one, etc. as long as the result is still within the representable range.

242

I'm not sure if this is necessary but I feel like it may be useful to permit adding a signed (negative value) to an unsigned value as long as the result is >= 0. Maybe these checks should move into operator+/-?

llvm/test/FileCheck/numeric-expression.txt

68

Since there are quite a lot of CHECK lines now, it might be helpful to add a comment after every line that checks a new variable. For example:

11 // VAR1
12
10
-30 // VAR2
-29
-31
c // VAR3 (hex)
...

[jhenderson]

Sorry for the delay in looking at this. Comments inline.

llvm/lib/Support/FileCheck.cpp

33

I'd not abbreviate this name. It's not clear to me that "Conv" == "Conversion"

Also, it's more idiomatic to declare the Error within the if, to reduce its scope.

66

other -> Other

67

I think you need a check that Signed == other.Signed, as otherwise there will be some cases where the two values match but aren't of the same signed-ness (e.g. -1 and uint64_t max). Unless that's intentional of course.

73

I'd make the names of this and convertUnsigned a bit more explicit, by adding "to", e.g. "toSigned" or "convertToSigned"

1149

As far as I can see, this block is just skipping the error? Why the switch/case?

llvm/lib/Support/FileCheckImpl.h

133 ↗

(On Diff #236825)

Perhaps a not_supported or invalid_argument would make more sense here.

158–162 ↗

(On Diff #236825)

This pair of constructors feels like a recipe for confusion or compilation problems due to ambiguous resolution. I think it might be better to make Signed an additional parameter.

166–167 ↗

(On Diff #236825)

other -> Other

225–229 ↗

(On Diff #236825)

As with above, this could lead to ambiguous compiler resolution, if Val is neither a uint64_t or int64_t. Perhaps not an issue in internal code, but it feels like there might be a better solution to me, perhaps using templates and std::is_signed in this case?

291 ↗

(On Diff #236825)

You've lost the reference to when this can be None. When can it be?

llvm/test/FileCheck/numeric-expression.txt

71

Maybe there should be som testing for signed positive values here?

[thopre]

Address review comments

[thopre]

llvm/lib/Support/FileCheck.cpp

1149

There's now only an OverflowError without type but FYI the goal was to catch the case of a new type of ValueError in the future that shouldn't be ignored here.

llvm/lib/Support/FileCheckImpl.h

133 ↗

(On Diff #236825)

From llvm/include/llvm/Support/Error.h:

/// The value returned by this function can be returned from convertToErrorCode
/// for Error values where no sensible translation to std::error_code exists.
/// It should only be used in this situation, and should never be used where a
/// sensible conversion to std::error_code is available, as attempts to convert
/// to/from this error will result in a fatal error. (i.e. it is a programmatic
///error to try to convert such a value).
std::error_code inconvertibleErrorCode();

That seems to be the right function to use when that function does not make sense for a given error?

[thopre]

Rebase

[thopre]

Last build failure were due to the pre commit check script trying to apply one of the earlier patch of this patch series as a needed dependency even though that diff is marked as closed.

[jhenderson]

llvm/docs/CommandGuide/FileCheck.rst

606

promotion rules

llvm/lib/Support/FileCheck.cpp

50–56

It might be worth a comment saying why signed plus unsigned results in an unsigned (of course, that assumes I'm following the logic correctly).

56

I'm a little confused as to what is going on here, as I'm not up-to-speed on APInt. Could you give a brief explanation, please?

79

Doesn't this assume 2s complement, which isn't standard-guaranteed as far as I'm aware? I'm okay with that, if there is precendent for it in LLVM already, since it seems like C++ is moving in that direction (see http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p0907r1.html), but I'd be nervous otherwise.

1054

Why do we handle OverflowError here, but not other error kinds (e.g. undefined variables)?

1056

Unable -> unable

(diagnostic messages should not be capitalized)

llvm/lib/Support/FileCheckImpl.h

87 ↗

(On Diff #263290)

Delete the "if" at the end of this line (it's not needed).

106 ↗

(On Diff #263290)

std::errc::value_too_large is for overflows, so maybe that can be used here?

109 ↗

(On Diff #263290)

This comment doesn't seem right?

110 ↗

(On Diff #263290)

Probably should be lower-case "overflow"

143 ↗

(On Diff #263290)

I'm slightly nervous "error in case of overflow" might go stale, in the event we introduce other failure modes. Perhaps "an error in case of a problem, such as an overflow."

llvm/test/FileCheck/numeric-expression.txt

44–48

Any particular reason you've changed this case?

82–84

Nit: here and below, add space after comment markers, i.e. // VAR1

Also, has this been addressed:

Maybe there should be som testing for signed positive values here?

152

Here and elsewhere there seem to be some extra cases added that aren't to do with signed decimal values. Do they belong in this change?

175–176

It seems like we need testing showing the behaviour of signed decimals converting to hex.

295–296

Here and elsewhere, minor nit: be consistent with single or double-dashes.

llvm/unittests/Support/FileCheckTest.cpp

108 ↗

(On Diff #263290)

MinusFortyTwo

385 ↗

(On Diff #263290)

What about 10u + -11 or -11 + 10u?

405 ↗

(On Diff #263290)

What about MinusTen - MinusTen?

461 ↗

(On Diff #263290)

10ull would be more typical, I'd think, here and elsewhere. Any particular reason you've chosen the cast?

[arichardson]

llvm/lib/Support/FileCheck.cpp

56

Using APInt here seems unnecessary. How about return (int64_t)Value;?

Or if you would like to avoid the casts you could use a union.

79

Since we know the value is signed after if (!Signed), can't we change these three lines to
return ExpressionValue(std::abs((int64_t)Value), /*Signed=*/false)?

llvm/lib/Support/FileCheckImpl.h

127 ↗

(On Diff #263290)

Signed && (int64_t)Value < 0?

[arichardson]

llvm/unittests/Support/FileCheckTest.cpp

461 ↗

(On Diff #263290)

ull is not guaranteed to be the same as a uint64_t constant since some platforms use unsigned long instead of unsigned long long. stdint.h provides UINT64_C(10) if you want to avoid the cast.

[thopre]

llvm/lib/Support/FileCheck.cpp

56

Value is uint64_t and C++14 says:

If the destination type is signed, the value is unchanged if it can be represented in the destination type;otherwise, the value is implementation-defined

So if Value corresponds to a negative signed integer the conversion is implementation-defined. I don't know how APInt deal with this but I hope it does something C++14 compliant.

79

As per above, the conversion to (int64_t) is implementation-defined in case where Value has its msb set (ie. corresponds to a negative signed value).

[thopre]

Address all comments

[thopre]

Fix casing

[MaskRay]

llvm/lib/Support/FileCheckImpl.h

46 ↗

(On Diff #263558)

a signed integer

[jhenderson]

llvm/test/FileCheck/numeric-expression.txt

87–88

I'm not following the purpose of the blank line here?

llvm/unittests/Support/FileCheckTest.cpp

385 ↗

(On Diff #263290)

Where has this been addressed?

405 ↗

(On Diff #263290)

Where has this been addressed?

[thopre]

llvm/lib/Support/FileCheckImpl.h

127 ↗

(On Diff #263290)

Casting to int64_t is implementation-defined if Value is > std::numeric_limits<int64_t>::max(). That said current code assumes 2-complement so I've changed for memcpy. Hopefully the compiler can elide that call.

llvm/test/FileCheck/numeric-expression.txt

82–84

Yes, that's the VAR2+72 case.

152

Ah yes indeed, I noticed some bug in the testcase. I forgot to split the fix out in its own patch. Now done in https://reviews.llvm.org/D79820

llvm/unittests/Support/FileCheckTest.cpp

385 ↗

(On Diff #263290)

Those are ExpressionValue since this test is testing ExpressionValue operator+. I've renamed all such variable to add the suffix "Value"

405 ↗

(On Diff #263290)

Ditto.

[thopre]

llvm/test/FileCheck/numeric-expression.txt

87–88

The CHECK-NEXT: #VAR2a is trying to match c which is also found in "// VAR1c". The CHECK-NEXT thus fails because it found c after the previous match on the same line.

llvm/unittests/Support/FileCheckTest.cpp

385 ↗

(On Diff #263290)

I forgot to send the comments I had written. I hope that answers why I didn't do it.

[arichardson]

llvm/lib/Support/FileCheckImpl.h

127 ↗

(On Diff #263290)

Yes it definitely can (even at -O1): https://godbolt.org/z/5jcz9L
I'm still not sure that it's possible that casting gives a different value from memcpy on any architecture that LLVM builds on, but I'm also fine with the memcpy.

[thopre]

Fix typo

[thopre]

The current promotion rules prevent to compute VAR-10 when VAR is an unsigned variable whose value is 5 because the result would be negative but the expression should be unsigned. I'll address that by accepting all values as long as they fit in an int64_t or uint64_t and only throw an overflow error when trying to match the value if it doesn't fit with the expression format. I should have done that in the first place.

[thopre]

llvm/lib/Support/FileCheckImpl.h

127 ↗

(On Diff #263290)

architecture -> system, surely the compiler can mess things up as well? It's such a shame that there isn't a better way to achieve this while being perfectly well defined.

[jhenderson]

llvm/test/FileCheck/numeric-expression.txt

87–88

I see. I'm beginning to think the names of the variables need improving. Rather than have VAR1, VAR2, VAR1a etc, how about "SIGN", "UNSI" "UHEX", "LHEX", etc, possibly with additional context for their actual values. The CHECKs might also want an additional {{^}} to avoid matching things spuriously.

llvm/unittests/Support/FileCheckTest.cpp

385 ↗

(On Diff #263290)

I'm not sure it does, if I'm honest. An unsigned 10 plus a signed -11 should trigger the clause at line 174 in FileCheck.cpp, whilst the other way around triggers the clause at 170. They are distinctly interesting from the other cases that trigger those clauses because they should trigger overflows, if I've understood things right, rather than resulting in max uint64_t, but I don't think that's tested directly. The fact that this happens to call into other functions seems somewhat irrelevant from testing a distinct unit perspective - if you refactored the code to no longer use the same code path as something else that might be tested independently, you still want the test coverage.

405 ↗

(On Diff #263290)

It seems to me like operator- is a distinct unit, and its interface should be fully tested independently (see also my comments above). I believe this ends path ends up calling into checkedSub for which I can see no direct testing (does it handle double-negatives correctly, for example?)

[thopre]

Address review comments

llvm/unittests/Support/FileCheckTest.cpp

385 ↗

(On Diff #263290)

I've reworked the ExpressionValue storage which should simplify the codepath somewhat and added one testcase to cover for an overflow case that was not tested (I was relying on testing of checkedSub but as you pointed out testing code would risk to bitrot in case the operator code is changed).

Please let me know if that doesn't address all your concerns.

405 ↗

(On Diff #263290)

How about now? Whether the value is signed or not doesn't matter anymore, only whether it's negative which should allow to reduce the amount of overflow codepaths.

[jhenderson]

llvm/docs/CommandGuide/FileCheck.rst

606–608

Is this still true?

llvm/lib/Support/FileCheck.cpp

105

I think there's an overflow issue here, right? If RightValue is bigger than max int64_t, it's possible for the expression to end up outside a representable int64_t, which seems like it should be an error?

llvm/lib/Support/FileCheckImpl.h

127–131 ↗

(On Diff #264003)

I'm looking at this and thinking can't this and the above constructor not just be changed to:

template <class T>
explicit ExpressionValue(T Val)
  : Value(Val), Negative(Val < 0) {}

133 ↗

(On Diff #264003)

true is -> true if

llvm/unittests/Support/FileCheckTest.cpp

385 ↗

(On Diff #263290)

I'm starting to get a bit of review fatigue, so I'm starting to miss things, but if I've followed things right, my suggested simplification to the constructors should enable us to get rid of the distinction between SignedTen and UnsignedTen, which in turn should allow the tests to be much simpler. At that point, I'd expect the test cases to be:

1. Negative + Positive
2. Positive + Negative
3. Negative + Negative (no overflow)
4. Positive + Positive (no overflow)

5-6) Overflowing variations of 3) and 4)

with an equivalent set for operator-

[thopre]

Address review comments and fix a few more bugs

llvm/lib/Support/FileCheck.cpp

47–48

Undefined on 2-complement system when Value == min int64_t.

83

This can fail since RightOperand, being positive, is represented by a uint64_t.

105

Indeed, and it made me realize that getAbsolute doesn't work for min int64_t either.

llvm/unittests/Support/FileCheckTest.cpp

385 ↗

(On Diff #263290)

My apologies for causing this fatigue. I've added comments for each individual checks for each of the function/operator tested. Hope this helps. I've also caught a bug in the -A -B case where B would not fit in an int64_t. There's now an extra underflow here with an extra test.

[thopre]

Fix format issue

[jhenderson]

I've been thinking more about test cases, and suggested a number more. If you can reduce the boiler plate for each individual test case, that would be great too. Perhaps even consider a TEST_P solution to specify input and expected output, whilst sharing the same code body.

llvm/unittests/Support/FileCheckTest.cpp

312 ↗

(On Diff #264382)

I'd test the range edge of -1 instead of -10.

It may be worth showing it fails for int64_t min too.

Also, a 0 check would be good to ensure no off-by-1 errors/incorrect use of < versus <= etc.

324–325 ↗

(On Diff #264382)

Perhaps worth adding a int64_t max + 1 case.

330 ↗

(On Diff #264382)

Maybe also int64_t min and max cases?

339 ↗

(On Diff #264382)

Maybe worth considering test cases for 0, and uint64_t max too?

368 ↗

(On Diff #264382)

Perhaps use -1 instead of -10.

Perhaps also MinInt64 - MinInt64.

373–377 ↗

(On Diff #264382)

These checks all have a lot of repeated code. It's probably time to factor out much of this into a function. It might also be a good idea to factor them into separate TESTs to allow the rest of the test to run in the event of a failure with an ASSERT etc, and to provide finer-grained test failures.

385 ↗

(On Diff #264382)

I'd prefer these slightly to not just always give a result of 0. I think it is more interesting to show that 10 + -11 can produce a negative number, for example, whilst 11 + -10 produces a positive one. Same goes for the case above.

396 ↗

(On Diff #264382)

Perhaps 1 instead of 10 here.

Perhaps also MaxUint64Value + MaxUint64Value would be a useful case, to maximise surface area.

401 ↗

(On Diff #264382)

What is meant by "too big" here? Be more precise in this comment.

406 ↗

(On Diff #264382)

- 1 rather than - 10 would probably be best here.

419 ↗

(On Diff #264382)

Similar to the + case, it might be nice for this to result in a non-zero value.

434 ↗

(On Diff #264382)

"or nul"?

435 ↗

(On Diff #264382)

I'm not sure Value is a useful suffix to all these names. It wasn't confusing to me before either.

444 ↗

(On Diff #264382)

Maybe Zero instead of Ten here would be good, for maximum possible underflow.
Maybe consider 0 - (std::abs(MinInt64) + 1) for minimum possible underflow.

455 ↗

(On Diff #264382)

Why not test the exact value?

458 ↗

(On Diff #264382)

Seems to me like there are two interesting cases here: "result is positive" and "result is negative".

[thopre]

Address unittest review comments

[thopre]

Fix format by using source tree clang-format-diff.py instead of system clang-format-diff

llvm/unittests/Support/FileCheckTest.cpp

373–377 ↗

(On Diff #264382)

Is that fine grain enough or do you want me to split further?

444 ↗

(On Diff #264382)

I can't use std::abs(MinInt64) since it's undefined in 2-complement (std::abs returns an int64_t for some reason).

458 ↗

(On Diff #264382)

Result is positive is already handled above. I meant to handle the negative case where result < -(max int64_t). Fixed now.

[jhenderson]

The latest changes are a big improvement on readability, thanks. Did you consider using TEST_P to factor things out further? That way you'd only need to specify the input and expected output values for each test case, with only a single TEST itself.

llvm/unittests/Support/FileCheckTest.cpp

298 ↗

(On Diff #265084)

Don't know how plausible it is, but did you consider passing in a function_ref to the operator+/operator- functions instead of the boolean? That might be useful in the future if support is extended to e.g. multiplication.

365 ↗

(On Diff #265084)

I think these sorts of comments indicate a need to split the test up. The suggestion I have is one method == one (or more) TESTs.

Thus there'd be tests for getAbsolute, getSignedValue etc etc and I'd call them ExpressionValueGetAbsolute, ExpressionValueGetSignedValue etc.

411 ↗

(On Diff #265084)

Typo

444 ↗

(On Diff #264382)

(Just to be clear, I only used std::abs as an intended example, rather than literally what should have been written).

[thopre]

Address inline review comments

[thopre]

In D60390#2048630, @jhenderson wrote:

The latest changes are a big improvement on readability, thanks. Did you consider using TEST_P to factor things out further? That way you'd only need to specify the input and expected output values for each test case, with only a single TEST itself.

To make the result more concise that would require sharing fixture and instantiation among TEST_P but I'm not sure how to do it since the values generated in the instantiation are passed to all TEST_P but even operator+ and operator- have different values. If I need separate instantiation per TEST_P then I need separate fixture as well and that becomes more faff that what we have now I feel.

If there's some clever trick please give me some pointers on how to do it as I cannot see it right now.

[jhenderson]

LGTM, with nit.

Re. TEST_P, the best suggestion I've got is look at the examples in DWARFDebugLineTest.cpp, where I've used a few different tricks to achieve the goal of code re-use. In particular, AdjustAddressFixtureBase related test cases might be interesting to you. In your case, I'd probably have a set of parameters each for failing and passing combinations, and for addition and subtraction, for a total of 4 combinations, if I'm not mistaken. You'd have thin fixtures for each, but they could probably share some code via a base class.

That all being said, only make those changes if you think they are worthwhile. I'm happy enough with this as-is.

llvm/unittests/Support/FileCheckTest.cpp

422 ↗

(On Diff #265602)

Here and in various other places, not sure you want the space between - and 10 (same for - 20 and various cases below).

[thopre]

llvm/unittests/Support/FileCheckTest.cpp

422 ↗

(On Diff #265602)

I don't like it either but this comes from clang-format-diff.py. Should I go against it?

[thopre]

In D60390#2056652, @jhenderson wrote:

LGTM, with nit.

Re. TEST_P, the best suggestion I've got is look at the examples in DWARFDebugLineTest.cpp, where I've used a few different tricks to achieve the goal of code re-use. In particular, AdjustAddressFixtureBase related test cases might be interesting to you. In your case, I'd probably have a set of parameters each for failing and passing combinations, and for addition and subtraction, for a total of 4 combinations, if I'm not mistaken. You'd have thin fixtures for each, but they could probably share some code via a base class.

That all being said, only make those changes if you think they are worthwhile. I'm happy enough with this as-is.

So far, I'm getting:

llvm/unittests/Support/FileCheckTest.cpp | 413 +++++++++++++++++++++++++++++++++-------------------
1 file changed, 263 insertions(+), 150 deletions(-)

It's also quite a lot of code change, so even if that is acceptable because the code is deemed more readable, it's better to look at it as a separate patch and land the current code (with the suggested spacing fix)

[thopre]

Rebase and fix spacing issue for negative literals

[thopre]

Argh sorry, forgot to remove the unnecessary phabricator marks when landing.

[jhenderson]

In D60390#2059448, @thopre wrote:

In D60390#2056652, @jhenderson wrote:

LGTM, with nit.

Re. TEST_P, the best suggestion I've got is look at the examples in DWARFDebugLineTest.cpp, where I've used a few different tricks to achieve the goal of code re-use. In particular, AdjustAddressFixtureBase related test cases might be interesting to you. In your case, I'd probably have a set of parameters each for failing and passing combinations, and for addition and subtraction, for a total of 4 combinations, if I'm not mistaken. You'd have thin fixtures for each, but they could probably share some code via a base class.

That all being said, only make those changes if you think they are worthwhile. I'm happy enough with this as-is.

So far, I'm getting:

llvm/unittests/Support/FileCheckTest.cpp | 413 +++++++++++++++++++++++++++++++++-------------------
1 file changed, 263 insertions(+), 150 deletions(-)

It's also quite a lot of code change, so even if that is acceptable because the code is deemed more readable, it's better to look at it as a separate patch and land the current code (with the suggested spacing fix)

I'll leave it up to you as to whether you want to. This is probably sufficient as is.

llvm/unittests/Support/FileCheckTest.cpp

422 ↗

(On Diff #265602)

If clang-format-diff.py was trying to format it that way, I'd consider that a bug in clang-format, so should be reported as such. The - there is a unary operator, and should be formatted as such (without a space).

llvm/docs/CommandGuide/FileCheck.rst

 values satisfying a numeric expression constraint based on those variables.
 This allows to capture numeric relations between two numbers in a text to
 check, such as the need for consecutive registers to be used.
 
 The syntax to define a numeric variable is ``[[#%<fmtspec>,<NUMVAR>:]]`` where:
 
 * ``%<fmtspec>`` is an optional scanf-style matching format specifier to
   indicate what number format to match (e.g. hex number).  Currently accepted
-  format specifier are ``%u``, ``%x`` and ``%X``.  If absent, the format
-  specifier defaults to ``%u``.
+  format specifier are ``%u``, ``%d``, ``%x`` and ``%X``.  If absent, the
+  format specifier defaults to ``%u``.
 
 * ``NUMVAR`` is the name of the numeric variable to define to the matching
   value.
 
 For example:
 
 .. code-block:: llvm
 
     ; CHECK: mov r[[#REG:]], 0x[[#%X,IMM:]]
 
 would match ``mov r5, 0xF0F0`` and set ``REG`` to the value ``5`` and ``IMM``
 to the value ``61680``.
 
 The syntax to check a numeric expression constraint is
 ``[[#%<fmtspec>:<expr>]]`` where:
 
 * ``%<fmtspec>`` is the same matching format specifier as for defining numeric
   variables but acting as a printf-style format to indicate how a numeric
   expression value should be matched against.  If absent, the format specifier
   is inferred from the matching format of the numeric variable(s) used by the
   expression constraint if any and they are all the same, and default to ``%u``
   if no numeric variable is used.  In case of conflict between matching formats
   of several numeric variables the format specifier is mandatory.
 
 * ``<expr>>`` is a numeric expression whose operands are either numeric
   variables previously defined or integer immediates.  Currently supported
   numeric operations are ``+`` and ``-``. A single numeric variable or
   integer literal is also accepted.
 
 For example:
 
 .. code-block:: llvm
 

llvm/include/llvm/Support/FileCheck.h

struct FileCheckRequest {
   bool Verbose = false;
   bool VerboseVerbose = false;
 };
 
 //===----------------------------------------------------------------------===//
 // Numeric expression handling code.
 //===----------------------------------------------------------------------===//
 
+class FileCheckNumExprVal;
+
 /// Bitfield representing the format a numeric expression value should be
 /// printed into for matching. Used to represent both explicit format
 /// specifiers as well as implicit format from using numeric variables.
 struct FileCheckNumExprFmt {
+  /// Value is a signed integer.
+  unsigned Signed : 1;
   /// Value should be printed as hex number.
   unsigned Hex : 1;
   /// Value should be printed using upper case letters, only used for hex
   /// numbers.
   unsigned Cap : 1;
 
   /// When unset, denote absence of format and thus that all of the other
   /// fields are to be ignored. Used for implicit format of literals and empty
struct FileCheckNumExprFmt {
   }
 
   /// Return wildcard regexp StringRef to match any value in the format
   /// represented by this instance.
   StringRef getWildcardRegex() const;
 
   /// Return the string representation of \p Value in the format represented by
   /// this instance.
-  std::string getMatchingString(uint64_t Value) const;
+  llvm::Optional<std::string>
+  getMatchingString(FileCheckNumExprVal &Value) const;
 
   /// Return the value corresponding to string representation \p StrVal
-  /// according to the matching format represented by this instance or nothing
-  /// if \p StrVal does not correspond to a valid and representable value.
-  llvm::Optional<uint64_t> valueFromStringRepr(StringRef StrVal) const;
-
+  /// according to the matching format represented by this instance or an
+  /// invalid value if \p StrVal does not correspond to a valid and
+  /// representable value.
+  FileCheckNumExprVal valueFromStringRepr(StringRef StrVal) const;
 };
 
 /// Initializer for numeric expression without format.
-const FileCheckNumExprFmt FmtNone = {0, 0, 0, 0};
+const FileCheckNumExprFmt FmtNone = {0, 0, 0, 0, 0};
 /// Initializer for numeric expression matched as unsigned value.
-const FileCheckNumExprFmt FmtUnsigned = {0, 0, 1, 0};
+const FileCheckNumExprFmt FmtUnsigned = {0, 0, 0, 1, 0};
+/// Initializer for numeric expression matched as signed value.
+const FileCheckNumExprFmt FmtSigned = {1, 0, 0, 1, 0};
 /// Initializer for numeric expression matched as lower case hex value.
-const FileCheckNumExprFmt FmtLowHex = {1, 0, 1, 0};
+const FileCheckNumExprFmt FmtLowHex = {0, 1, 0, 1, 0};
 /// Initializer for numeric expression matched as capital case hex value.
-const FileCheckNumExprFmt FmtCapHex = {1, 1, 1, 0};
+const FileCheckNumExprFmt FmtCapHex = {0, 1, 1, 1, 0};
+
+/// Class representing a numeric value.
+class FileCheckNumExprVal {
+private:
+  union {
+    int64_t SignedValue;
+    uint64_t UnsignedValue;
+  };
+
+  /// Whether value is signed (and thus is stored in SignedValue) or not (in
+  /// which case it is stored in UnsignedValue).
+  bool Signed;
+
+  /// Whether this holds an actual value. Examples of where this would be
+  /// false:
+  /// - underflow or overflow of one of the binary operation in the expression;
+  /// - unset variable's value.
+  bool Valid;
+
+public:
+  /// Constructor for an invalid value.
+  FileCheckNumExprVal() : Valid(false) {}
+
+  /// Constructor for a signed value.
+  FileCheckNumExprVal(int64_t Val)
+      : SignedValue(Val), Signed(true), Valid(true) {}
+
+  /// Constructor for an unsigned value.
+  FileCheckNumExprVal(uint64_t Val)
+      : UnsignedValue(Val), Signed(false), Valid(true) {}
+
+  /// Define equality to be true only if both values are valid and they have
+  /// the same signedness and corresponding value.  Tentative bit is ignored to
+  /// allow the evaluation of a numeric expression using variables with
+  /// tentative value to compare equal to a matched value.
+  bool operator==(const FileCheckNumExprVal &other);
+  bool operator!=(const FileCheckNumExprVal &other) {
+    return !(*this == other);
+  }
+
+  bool isSigned() const { return Signed; }
+
+  bool isValid() const { return Valid; }
+
+  /// Return the signed value. Must only be called if value is signed in the
+  /// first place.
+  int64_t getSignedValue() const {
+    assert(Signed);
+    return SignedValue;
+  }
+
+  /// Return the unsigned value. Must only be called if value is unsigned in
+  /// the first place.
+  uint64_t getUnsignedValue() const {
+    assert(!Signed);
+    return UnsignedValue;
+  }
+
+  /// Convert value to a signed value, or mark value invalid if not possible
+  /// (original value was not within range for a signed integer).
+  void convertSigned();
+
+  /// Convert value to an unsigned value, or mark value invalid if not possible
+  /// (original value was not within range for an unsigned integer).
+  void convertUnsigned();
+
+  /// Return an invalid value in case of underflow or overflow.
+  friend FileCheckNumExprVal operator+(const FileCheckNumExprVal &lhs,
+                                       const FileCheckNumExprVal &rhs);
+  friend FileCheckNumExprVal operator-(const FileCheckNumExprVal &lhs,
+                                       const FileCheckNumExprVal &rhs);
+};
 
 /// Base class representing the AST of a given numeric expression.
 class FileCheckNumExprAST {
 public:
   virtual ~FileCheckNumExprAST() = default;
 
   /// Evaluate the value of the expression represented by this AST.
-  virtual llvm::Optional<uint64_t> eval() const = 0;
+  virtual FileCheckNumExprVal eval() const = 0;
 
   /// Return implicit format of this AST, FmtConflict if implicit formats of
   /// the AST's components conflict and Fmt none if the AST has no implicit
   /// format (e.g. AST is made of a single literal).
   virtual FileCheckNumExprFmt getImplicitFmt() const = 0;
 
   /// Append names of undefined variables used in the expression represented by
   /// this AST. Must be overriden in any subclass representing an expression
   /// that can contain a variable.
   virtual void
   appendUndefVarNames(std::vector<StringRef> &UndefVarNames) const {}
 };
 
 /// Class representing a literal in the AST of a numeric expression.
 class FileCheckNumExprLiteral : public FileCheckNumExprAST {
 private:
   /// Actual value of the literal.
-  uint64_t Value;
+  FileCheckNumExprVal Value;
 
 public:
+  /// Constructor for a signed literal.
+  FileCheckNumExprLiteral(int64_t Val) : Value(Val) {}
+
   /// Constructor for an unsigned literal.
   FileCheckNumExprLiteral(uint64_t Val) : Value(Val) {}
 
   /// Evaluate the value of the expression represented by this AST. Therefore,
   /// return the literal's value.
-  llvm::Optional<uint64_t> eval() const { return Value; }
+  FileCheckNumExprVal eval() const { return Value; }
 
   /// Return implicit format of this AST, therefore FmtNone.
   FileCheckNumExprFmt getImplicitFmt() const { return FmtNone; }
 };
 
 /// Class representing a numeric expression and its matching format.
 class FileCheckNumExpr {
 private:
private:
   StringRef Name;
 
   /// Pointer to numeric expression defining this numeric variable. Only null
   /// for pseudo variable whose value is known at parse time (e.g. @LINE pseudo
   /// variable). If numeric expression is empty NumExpr points to a
   /// FileCheckNumExpr with a null AST.
   std::shared_ptr<FileCheckNumExpr> NumExpr;
 
-  /// Whether variable is defined and thus Value is set.
-  bool Defined;
-
-  /// Value of numeric variable if defined.
-  uint64_t Value;
+  /// Value of numeric variable.
+  FileCheckNumExprVal Value;
 
   /// Line number where this variable is defined. Used to determine whether a
   /// variable is defined on the same line as a given use.
   unsigned DefLineNumber;
 
 public:
   /// Constructor for a variable \p Name defined at line \p DefLineNumber to
   /// the numeric expression represented by NumExpr.
   FileCheckNumExprVar(StringRef Name, std::shared_ptr<FileCheckNumExpr> NumExpr,
                       unsigned DefLineNumber)
-      : Name(Name), NumExpr(NumExpr), Defined(false),
-        DefLineNumber(DefLineNumber) {}
+      : Name(Name), NumExpr(NumExpr), DefLineNumber(DefLineNumber) {
+    Value = FileCheckNumExprVal();
+  }
 
   /// Constructor for numeric variable \p Name with a known \p Value at parse
   /// time (e.g. the @LINE numeric variable).
-  FileCheckNumExprVar(StringRef Name, uint64_t Value)
-      : Name(Name), NumExpr(nullptr), Defined(true), Value(Value) {}
+  FileCheckNumExprVar(StringRef Name, FileCheckNumExprVal &Value)
+      : Name(Name), NumExpr(nullptr), Value(Value) {}
 
   /// Return name of that numeric variable.
   StringRef getName() const { return Name; }
 
   /// Return numeric expression associated with this numeric variable.
   std::shared_ptr<FileCheckNumExpr> getNumExpr() const { return NumExpr; }
 
   /// Evaluate the value of the expression represented by this AST. Therefore,
   /// return this variable's value or the value of its associated numeric
   /// expression if any.
-  llvm::Optional<uint64_t> eval() const;
+  FileCheckNumExprVal eval() const;
 
   /// Return whether this variable's value is known at match time, when
   /// performing the substitutions.
   bool isMatchTimeKnown() const;
 
   /// Return implicit format of this numeric variable.
   FileCheckNumExprFmt getImplicitFmt() const;
 
   /// Append numeric variable's name to UndefVarNames if undefined.
   void appendUndefVarNames(std::vector<StringRef> &UndefVarNames) const;
 
   /// Set value of this numeric variable if not defined. Return whether
   /// variable was already defined.
-  bool setValue(uint64_t Value);
+  bool setValue(FileCheckNumExprVal Value);
 
   /// Clear value of this numeric variable. Return whether value was already
   /// undefined.
   bool clearValue();
 
   /// Return line number where this variable is defined.
   unsigned getDefLineNumber() { return DefLineNumber; }
 };
 
 /// Type of functions evaluating a given binary operation.
-using binop_eval_t = uint64_t (*)(uint64_t, uint64_t);
+using binop_eval_t = FileCheckNumExprVal (*)(const FileCheckNumExprVal &,
+                                             const FileCheckNumExprVal &);
 
 /// Class representing a single binary operation in the AST of a numeric
 /// expression.
 class FileCheckASTBinop : public FileCheckNumExprAST {
 private:
   /// Left operand.
   std::shared_ptr<FileCheckNumExprAST> Opl;
 
   /// Right operand.
   std::shared_ptr<FileCheckNumExprAST> Opr;
 
   /// Pointer to function that can evaluate this binary operation.
   binop_eval_t EvalBinop;
 
 public:
   FileCheckASTBinop(binop_eval_t EvalBinop,
                     std::shared_ptr<FileCheckNumExprAST> OperandLeft,
                     std::shared_ptr<FileCheckNumExprAST> OperandRight)
       : Opl(OperandLeft), Opr(OperandRight), EvalBinop(EvalBinop) {}
 
   /// Evaluate the value of the binary operation represented by this AST. Uses
   /// EvalBinop to perform the binary operation on the values of recursively
   /// evaluating the left and right operands.
-  llvm::Optional<uint64_t> eval() const;
+  FileCheckNumExprVal eval() const;
 
   /// Return implicit format of this AST, FmtConflict if implicit formats of
   /// the AST's components conflict and Fmt none if the AST has no implicit
   /// format (e.g. AST is made of a single literal).
   FileCheckNumExprFmt getImplicitFmt() const;
 
   /// Append names of undefined variables used in any of the operands of this
   /// binary operation.

llvm/lib/Support/FileCheck.cpp

 #include <map>
 #include <tuple>
 #include <utility>
 
 using namespace llvm;
 
 /// Define format equality: formats are equal if all bits are identical.
 bool FileCheckNumExprFmt::operator==(const FileCheckNumExprFmt &other) {
-  return Valid == other.Valid && Hex == other.Hex && Cap == other.Cap;
+  return Valid == other.Valid && Conflict == other.Conflict &&
+         Signed == other.Signed && Hex == other.Hex && Cap == other.Cap;
 }
 
 /// Return wildcard regexp StringRef to match any value in the format
 /// represented by this instance.
 StringRef FileCheckNumExprFmt::getWildcardRegex() const {
   assert(Valid && !Conflict && "Trying to match value with invalid format");
   if (Hex) {
     if (Cap)
       return StringRef("[[:digit:]A-F]+");
     else
       return StringRef("[[:digit:]a-f]+");
-  } else
+  } else if (Signed)
+    return StringRef("-?[[:digit:]]+");
+  else
     return StringRef("[[:digit:]]+");
 }
 
 /// Return the string representation of \p Value in the format represented by
 /// this instance.
-std::string FileCheckNumExprFmt::getMatchingString(uint64_t Value) const {
-  assert(Valid && !Conflict && "Trying to match value with invalid format");
+llvm::Optional<std::string>
+FileCheckNumExprFmt::getMatchingString(FileCheckNumExprVal &Value) const {
+  if (Signed)
+    Value.convertSigned();
+  else
+    Value.convertUnsigned();
+
+  assert(!Conflict);
+  // Conversion error (e.g. negative value converted to unsigned).
+  if (!Valid)
+    return llvm::None;
+
   if (Hex)
-    return utohexstr(Value, !Cap);
+    return utohexstr(Value.getUnsignedValue(), !Cap);
+  else if (Signed)
+    return itostr(Value.getSignedValue());
   else
-    return utostr(Value);
+    return utostr(Value.getUnsignedValue());
 }
 
 /// Return the value corresponding to string representation \p StrVal according
 /// to the matching format represented by this instance or nothing if \p StrVal
 /// does not correspond to a valid and representable value.
-llvm::Optional<uint64_t>
+FileCheckNumExprVal
 FileCheckNumExprFmt::valueFromStringRepr(StringRef StrVal) const {
   unsigned Radix = Hex ? 16 : 10;
-  uint64_t Value;
+  if (Signed) {
+    int64_t SignedValue;
 
-  if (StrVal.getAsInteger(Radix, Value))
-    return llvm::None;
+    if (StrVal.getAsInteger(Radix, SignedValue))
+      return FileCheckNumExprVal();
 
-  return Value;
+    return FileCheckNumExprVal(SignedValue);
+  } else {
+    uint64_t UnsignedValue;
+
+    if (StrVal.getAsInteger(Radix, UnsignedValue))
+      return FileCheckNumExprVal();
+
+    return FileCheckNumExprVal(UnsignedValue);
+  }
+}
+
+/// Equality operator: 2 values are equal if both are valid and have same
+/// signedness and corresponding value. Tentative bit is ignored to allow the
+/// evaluation of a numeric expression using variables with tentative value to
+/// compare equal to a matched value.
+bool FileCheckNumExprVal::operator==(const FileCheckNumExprVal &other) {
+  if (!Valid || !other.Valid)
+    return false;
+
+  if (Signed)
+    return SignedValue == other.SignedValue;
+  else
+    return UnsignedValue == other.UnsignedValue;
+}
+
+/// Convert value to a signed value, or mark value invalid if not possible
+/// (original value was not within range for a signed integer).
+void FileCheckNumExprVal::convertSigned() {
+  if (!Valid || Signed)
+    return;
+
+  if (UnsignedValue > std::numeric_limits<int64_t>::max()) {
+    Valid = false;
+    return;
+  }
+
+  SignedValue = UnsignedValue;
+  Signed = true;
+}
+
+/// Convert value to an unsigned value, or mark value invalid if not possible
+/// (original value was not within range for an unsigned integer).
+void FileCheckNumExprVal::convertUnsigned() {
+  if (!Valid || !Signed)
+    return;
+
+  if (SignedValue < 0) {
+    Valid = false;
+    return;
+  }
+
+  UnsignedValue = SignedValue;
+  Signed = false;
+  return;
+}
+
+/// Return an invalid value in case of underflow or overflow.
+llvm::FileCheckNumExprVal llvm::
+operator+(const llvm::FileCheckNumExprVal &Op1,
+          const llvm::FileCheckNumExprVal &Op2) {
+  // Operands must be valid.
+  if (!Op1.Valid || !Op2.Valid)
+    return FileCheckNumExprVal();
+
+  // Operands must have same sign.
+  if (Op1.Signed != Op2.Signed)
+    return FileCheckNumExprVal();
+
+  if (Op1.Signed) {
+    int64_t Val1 = Op1.SignedValue;
+    int64_t Val2 = Op2.SignedValue;
+
+    // Op1 + Op2 > max int64_t.
+    if (Val1 > 0 && Val2 > 0 &&
+        Val1 > (std::numeric_limits<int64_t>::max() - Val2))
+      return FileCheckNumExprVal();
+
+    // Op1 + Op2 < min int64_t.
+    if (Val1 < 0 && Val2 < 0 &&
+        Val1 < (std::numeric_limits<int64_t>::min() - Val2))
+      return FileCheckNumExprVal();
+
+    return FileCheckNumExprVal(Val1 + Val2);
+  } else {
+    uint64_t Val1 = Op1.UnsignedValue;
+    uint64_t Val2 = Op2.UnsignedValue;
+
+    // Op1 + Op2 > max uint64_t.
+    if (Val1 > std::numeric_limits<uint64_t>::max() - Val2)
+      return FileCheckNumExprVal();
+
+    return FileCheckNumExprVal(Val1 + Val2);
+  }
+}
+
+/// Return an invalid value in case of underflow or overflow.
+llvm::FileCheckNumExprVal llvm::
+operator-(const llvm::FileCheckNumExprVal &Op1,
+          const llvm::FileCheckNumExprVal &Op2) {
+  // Operands must be valid.
+  if (!Op1.Valid || !Op2.Valid)
+    return FileCheckNumExprVal();
+
+  // Operands must have same sign.
+  if (Op1.Signed != Op2.Signed)
+    return FileCheckNumExprVal();
+
+  if (Op1.Signed) {
+    int64_t Val1 = Op1.SignedValue;
+    int64_t Val2 = Op2.SignedValue;
+
+    // Op1 - Op2 > max int64_t.
+    if (Val1 > 0 && Val2 < 0 &&
+        Val1 > (std::numeric_limits<int64_t>::max() + Val2))
+      return FileCheckNumExprVal();
+
+    // Op1 - Op2 < min int64_t.
+    if (Val1 < 0 && Val2 > 0 &&
+        Val1 < (std::numeric_limits<int64_t>::min() + Val2))
+      return FileCheckNumExprVal();
+
+    return FileCheckNumExprVal(Val1 - Val2);
+  } else {
+    uint64_t Val1 = Op1.UnsignedValue;
+    uint64_t Val2 = Op2.UnsignedValue;
+
+    // Op1 < Op2.
+    if (Val1 < Val2)
+      return FileCheckNumExprVal();
+    return FileCheckNumExprVal(Val1 - Val2);
+  }
 }
 
 /// Generic constructor for a numeric expression whose equality constraint is
 /// represented by \p AST and matching format is \p Fmt. If matching format is
 /// unset (ie. no explicit or implicit matching format), set it to default one
 /// (unsigned decimal integer).
 FileCheckNumExpr::FileCheckNumExpr(std::shared_ptr<FileCheckNumExprAST> AST,
                                    FileCheckNumExprFmt Fmt)
     : AST(AST) {
   if (Fmt.Valid)
     this->Fmt = Fmt;
   else
     this->Fmt = FmtUnsigned;
 }
 
 /// Evaluate the value of the expression represented by this AST. Therefore,
 /// return this variable's value or the value of its associated numeric
 /// expression if any.
-llvm::Optional<uint64_t> FileCheckNumExprVar::eval() const {
-  if (Defined)
+FileCheckNumExprVal FileCheckNumExprVar::eval() const {
+  if (Value.isValid())
     return Value;
 
   assert(NumExpr != nullptr);
   if (NumExpr->getAST() != nullptr)
     return NumExpr->getAST()->eval();
 
-  return llvm::None;
+  return FileCheckNumExprVal();
 }
 
 /// Return whether this variable's value is known at match time, when
 /// performing the substitutions.
 bool FileCheckNumExprVar::isMatchTimeKnown() const {
   return NumExpr != nullptr && NumExpr->getAST() != nullptr;
 }
 
 /// Return implicit format of this numeric variable.
 FileCheckNumExprFmt FileCheckNumExprVar::getImplicitFmt() const {
   if (NumExpr == nullptr)
     return FmtNone;
 
   return NumExpr->getEffectiveFmt();
 }
 
 /// Append numeric variable's name to UndefVarNames if undefined.
 void FileCheckNumExprVar::appendUndefVarNames(
     std::vector<StringRef> &UndefVarNames) const {
-  if (!Defined)
+  if (!Value.isValid())
     UndefVarNames.emplace_back(Name);
 }
 
 /// Set value of this numeric variable if not defined. Return whether the
 /// variable was already defined.
-bool FileCheckNumExprVar::setValue(uint64_t Value) {
-  if (Defined)
+bool FileCheckNumExprVar::setValue(FileCheckNumExprVal Value) {
+  if (this->Value.isValid())
     return true;
+  assert(Value.isValid() && "Setting invalid value");
   this->Value = Value;
-  this->Defined = true;
   return false;
 }
 
 /// Clear value of this numeric variable. Return whether value was already
 /// undefined.
 bool FileCheckNumExprVar::clearValue() {
-  if (!Defined)
+  if (!Value.isValid())
     return true;
-  Defined = false;
+  Value = FileCheckNumExprVal();
   return false;
 }
 
 /// Evaluate the value of the binary operation represented by this AST. Uses
 /// EvalBinop to perform the binary operation on the values of recursively
 /// evaluating the left and right operands.
-llvm::Optional<uint64_t> FileCheckASTBinop::eval() const {
-  llvm::Optional<uint64_t> OptOpl = Opl->eval();
-  llvm::Optional<uint64_t> OptOpr = Opr->eval();
+FileCheckNumExprVal FileCheckASTBinop::eval() const {
+  FileCheckNumExprVal LVal = Opl->eval();
+  FileCheckNumExprVal RVal = Opr->eval();
 
-  // Uses undefined variable.
-  if (!OptOpl.hasValue())
-    return llvm::None;
-  if (!OptOpr.hasValue())
-    return llvm::None;
+  // Integer promotion.
+  if (!LVal.isSigned() || !RVal.isSigned()) {
+    LVal.convertUnsigned();
+    RVal.convertUnsigned();
+  }
+
+  if (!LVal.isValid() || !RVal.isValid())
+    return FileCheckNumExprVal();
 
-  return EvalBinop(OptOpl.getValue(), OptOpr.getValue());
+  return EvalBinop(LVal, RVal);
 }
 
 /// Return implicit format of this AST, FmtConflict if implicit formats of the
 /// AST's components conflict and Fmt none if the AST has no implicit format
 /// (e.g. AST is made of a single literal).
 FileCheckNumExprFmt FileCheckASTBinop::getImplicitFmt() const {
   FileCheckNumExprFmt LFmt = Opl->getImplicitFmt();
   FileCheckNumExprFmt RFmt = Opr->getImplicitFmt();

 /// nothing if substitution failed. For a numeric variable we substitute it by
 /// its value if known at match time or a suitable wildcard pattern otherwise.
 /// For a pattern variable we simply replace it by the text its definition
 /// matched.
 llvm::Optional<std::string> FileCheckPatternSubst::getSubstitute() const {
   if (IsNumExpr) {
     assert(NumExpr->getAST() != nullptr &&
            "Substituting empty numeric expression");
-    llvm::Optional<uint64_t> EvaluatedValue = NumExpr->getAST()->eval();
-    if (!EvaluatedValue.hasValue())
+    FileCheckNumExprVal EvaluatedValue = NumExpr->getAST()->eval();
+    if (!EvaluatedValue.isValid())
       return llvm::None;
+
     FileCheckNumExprFmt Fmt = NumExpr->getEffectiveFmt();
-    return Fmt.getMatchingString(EvaluatedValue.getValue());
+    return Fmt.getMatchingString(EvaluatedValue);
   } else {
     // Look up the value and escape it so that we can put it into the
     // regex.
     llvm::Optional<StringRef> OptVarVal;
     OptVarVal = Context->getPatternVarValue(SubstStr);
     if (!OptVarVal.hasValue())
       return llvm::None;
     return Regex::escape(OptVarVal.getValue());
if (!parseNumericVariable(Expr, Name, false /*IsDefinition*/,
       return NumVar;
     }
   }
 
   // Otherwise, parse it as a literal.
   if (AO != LegacyLiteral && AO != Any)
     return nullptr;
   unsigned Radix = (AO == LegacyLiteral) ? 10 : 0;
-  uint64_t LiteralValue;
-  if (Expr.consumeInteger(Radix, LiteralValue))
+  int64_t SignedLiteralValue;
+  uint64_t UnsignedLiteralValue;
+  StringRef SaveExpr = Expr;
+  // Accept both signed and unsigned literal.
+  if (AO != LegacyLiteral && !Expr.consumeInteger(Radix, SignedLiteralValue)) {
+    return std::make_shared<FileCheckNumExprLiteral>(SignedLiteralValue);
+  } else {
+    Expr = SaveExpr;
+    if (!Expr.consumeInteger(Radix, UnsignedLiteralValue)) {
+      return std::make_shared<FileCheckNumExprLiteral>(UnsignedLiteralValue);
+    } else
       return nullptr;
-  return std::make_shared<FileCheckNumExprLiteral>(LiteralValue);
   }
-
-static uint64_t doAdd(uint64_t Opl, uint64_t Opr) { return Opl + Opr; }
-
-static uint64_t doSub(uint64_t Opl, uint64_t Opr) { return Opl - Opr; }
+}
 
 /// Parse \p Expr for a binary operation. The left operand of this binary
 /// operation is given in \p Opl and \p Legacy indicates whether we are parsing
 /// a legacy numeric expression. Return the class representing this binary
 /// operation in the AST of the numeric expression or nullptr if parsing fails
 /// in which case errors are reported on \p SM.
 std::shared_ptr<FileCheckNumExprAST>
 FileCheckPattern::parseFileCheckBinop(StringRef &Expr,
                                       std::shared_ptr<FileCheckNumExprAST> Opl,
                                       bool Legacy, const SourceMgr &SM) const {
   skipWhitespace(Expr);
   if (Expr.empty())
     return Opl;
 
   // Check if this is a supported operation and selection function to perform
   // it.
   SMLoc oploc = SMLoc::getFromPointer(Expr.data());
   char Operator = next(Expr);
   binop_eval_t EvalBinop;
   switch (Operator) {
   case '+':
-    EvalBinop = doAdd;
+    EvalBinop = operator+;
     break;
   case '-':
-    EvalBinop = doSub;
+    EvalBinop = operator-;
     break;
   default:
     SM.PrintMessage(oploc, SourceMgr::DK_Error,
                     std::string("Unsupported numeric operation '") + Operator +
                         "'");
     return nullptr;
   }
 
if (FmtSpecEnd != StringRef::npos) {
 
     // Check for unknown matching format specifier and set matching format in
     // class instance representing this numeric expression.
     SMLoc fmtloc = SMLoc::getFromPointer(Expr.data());
     switch (next(Expr)) {
     case 'u':
       ExplicitFmt = FmtUnsigned;
       break;
+    case 'd':
+      ExplicitFmt = FmtSigned;
+      break;
     case 'x':
       ExplicitFmt = FmtLowHex;
       break;
     case 'X':
       ExplicitFmt = FmtCapHex;
       break;
     default:
       SM.PrintMessage(fmtloc, SourceMgr::DK_Error,
bool FileCheckPattern::ParsePattern(StringRef PatternStr, StringRef Prefix,
                                     const FileCheckRequest &Req) {
   bool MatchFullLinesHere = Req.MatchFullLines && CheckTy != Check::CheckNot;
 
   PatternLoc = SMLoc::getFromPointer(PatternStr.data());
 
   // Create fake @LINE pseudo variable definition.
   StringRef LinePseudo = "@LINE";
   uint64_t LineNumber64 = this->LineNumber;
+  auto LineNumberVal = FileCheckNumExprVal(LineNumber64);
   auto LinePseudoVar =
-      std::make_shared<FileCheckNumExprVar>(LinePseudo, LineNumber64);
+      std::make_shared<FileCheckNumExprVar>(LinePseudo, LineNumberVal);
   Context->GlobalNumericVariableTable[LinePseudo] = LinePseudoVar;
 
   if (!(Req.NoCanonicalizeWhiteSpace && Req.MatchFullLines))
     // Ignore trailing whitespace.
     while (!PatternStr.empty() &&
            (PatternStr.back() == ' ' || PatternStr.back() == '\t'))
       PatternStr = PatternStr.substr(0, PatternStr.size() - 1);
 
if (!Substs.empty()) {
 
     unsigned InsertOffset = 0;
     // Substitute all pattern variables and numeric expressions whose value is
     // known just now. Use of pattern variables defined on the same line are
     // handled by back-references.
     for (const auto &Subst : Substs) {
       // Substitute and check for failure (e.g. use of undefined variable).
       llvm::Optional<std::string> OptValue = Subst.getSubstitute();
-      if (!OptValue.hasValue())
+      if (!OptValue.hasValue()) {
+        SM.PrintMessage(SMLoc::getFromPointer(Subst.getSubstString().data()),
+                        SourceMgr::DK_Error,
+                        "Unable to substitute variable or numeric expression");
         return StringRef::npos;
+      }
       std::string Value = OptValue.getValue();
 
       // Plop it into the regex at the adjusted offset.
       TmpStr.insert(TmpStr.begin() + Subst.getIndex() + InsertOffset,
                     Value.begin(), Value.end());
       InsertOffset += Value.size();
     }
 
assert(NumericVariableDef.second.CaptureParen < MatchInfo.size() &&
            "Internal paren error");
     unsigned CaptureParen = NumericVariableDef.second.CaptureParen;
     std::shared_ptr<FileCheckNumExprVar> NumVarDef =
         NumericVariableDef.second.NumVarDef;
 
     StringRef MatchedValue = MatchInfo[CaptureParen];
     assert(NumVarDef->getNumExpr() != nullptr);
     FileCheckNumExprFmt Fmt = NumVarDef->getNumExpr()->getEffectiveFmt();
-    llvm::Optional<uint64_t> OptValue = Fmt.valueFromStringRepr(MatchedValue);
-    if (!OptValue.hasValue()) {
+    FileCheckNumExprVal Value = Fmt.valueFromStringRepr(MatchedValue);
+    if (!Value.isValid()) {
       SM.PrintMessage(SMLoc::getFromPointer(MatchedValue.data()),
                       SourceMgr::DK_Error, "Unable to represent numeric value");
     }
-    if (NumVarDef->setValue(OptValue.getValue()))
+    if (NumVarDef->setValue(Value))
       assert(false && "Numeric variable redefined");
   }
 
   // Like CHECK-NEXT, CHECK-EMPTY's match range is considered to start after
   // the required preceding newline, which is consumed by the pattern in the
   // case of CHECK-EMPTY but not CHECK-NEXT.
   size_t MatchStartSkip = CheckTy == Check::CheckEmpty;
   MatchLen = FullMatch.size() - MatchStartSkip;
if (!Substs.empty()) {
     for (const auto &Subst : Substs) {
       SmallString<256> Msg;
       raw_svector_ostream OS(Msg);
       bool IsNumExpr = Subst.isNumExpr();
       llvm::Optional<std::string> OptMatchedValue = Subst.getSubstitute();
 
       // Substitution failed or is not known at match time, print undefined
       // variables it uses.
       if (!OptMatchedValue.hasValue()) {
         std::vector<StringRef> UndefVarNames;
         Subst.getUndefVarNames(UndefVarNames);
         if (UndefVarNames.empty())
           continue;
         for (auto UndefVarName : UndefVarNames) {
           OS << "uses undefined variable \"";
           OS.write_escaped(UndefVarName) << "\"";
         }
if (CmdlineDef[0] == '#') {
           DiagCmdlineDefRef.substr(NumExprStartIdx).drop_back(Suffix3.size());
       std::shared_ptr<FileCheckNumExprVar> NumVarDef;
       std::shared_ptr<FileCheckNumExpr> NumExpr =
           P.parseNumericExpression(CmdlineDefRef, NumVarDef, false, SM);
       if (NumExpr == nullptr || NumVarDef == nullptr) {
         ErrorFound = true;
         continue;
       }
-      llvm::Optional<uint64_t> OptValue = NumExpr->getAST()->eval();
-      if (!OptValue.hasValue()) {
+      FileCheckNumExprVal Value = NumExpr->getAST()->eval();
+      if (!Value.isValid()) {
         SM.PrintMessage(SMLoc::getFromPointer(CmdlineDefRef.data()),
                         SourceMgr::DK_Error,
                         "Unable to represent numeric value");
         ErrorFound = true;
         continue;
       }
-      NumVarDef->setValue(OptValue.getValue());
+      NumVarDef->setValue(Value);
 
       // Record this variable definition.
       GlobalNumericVariableTable[NumVarDef->getName()] = NumVarDef;
     } else {
       // Pattern variable definition
       std::string Prefix = "-D";
       std::string DiagCmdline = Prefix + CmdlineDef;
       std::unique_ptr<MemoryBuffer> CmdLine =

llvm/test/FileCheck/numeric-expression.txt

 ; RUN: FileCheck -input-file %s %s
 ; RUN: not FileCheck -check-prefixes ERR,INVALID-FMT-SPEC1 -input-file %s %s 2>&1 | FileCheck -check-prefix INVALID-FMT-SPEC-MSG1 %s
 ; RUN: not FileCheck -check-prefixes ERR,INVALID-FMT-SPEC2 -input-file %s %s 2>&1 | FileCheck -check-prefix INVALID-FMT-SPEC-MSG2 %s
 ; RUN: not FileCheck -check-prefix UNDEF-USE -input-file %s %s 2>&1 | FileCheck --strict-whitespace -check-prefix UNDEF-USE-MSG %s
 ; RUN: not FileCheck -check-prefixes CHECK,INVAL-OP -input-file %s %s 2>&1 | FileCheck --strict-whitespace -check-prefix INVAL-OP-MSG %s
 ; RUN: not FileCheck -check-prefixes CONFLICT,CONFLICT1,CONFLICT2 -input-file %s %s 2>&1 | FileCheck --strict-whitespace -check-prefix INPUT-PAT-CONFLICT %s
 ; RUN: not FileCheck -D#NUMVAR=42 -check-prefixes CONFLICT,CONFLICT2 -input-file %s %s 2>&1 | FileCheck --strict-whitespace -check-prefix INPUT-PAT-CONFLICT %s
 ; RUN: not FileCheck -D#NUMVAR=42 -DNUMVAR=foobar -check-prefix CONFLICT -input-file %s %s 2>&1 | FileCheck --strict-whitespace -check-prefix CLI-PAT-CONFLICT %s
 ; RUN: not FileCheck -check-prefixes CONFLICT,CONFLICT3,CONFLICT4 -input-file %s %s 2>&1 | FileCheck --strict-whitespace -check-prefix INPUT-NUM-CONFLICT %s
 ; RUN: not FileCheck -DPATVAR=foobar -check-prefixes CONFLICT,CONFLICT4 -input-file %s %s 2>&1 | FileCheck --strict-whitespace -check-prefix INPUT-NUM-CONFLICT %s
 ; RUN: not FileCheck -DPATVAR=foobar -D#PATVAR=42 -check-prefix CONFLICT -input-file %s %s 2>&1 | FileCheck --strict-whitespace -check-prefix CLI-NUM-CONFLICT %s
 ; RUN: not FileCheck -check-prefix DEF-EXPR-FAIL -input-file %s %s
 ; RUN: not FileCheck -check-prefixes CHECK,FMT-CONFLICT -input-file %s %s 2>&1 | FileCheck --strict-whitespace -check-prefix FMT-CONFLICT-MSG %s
+; RUN: not FileCheck -check-prefix OVERFLOW -input-file %s %s 2>&1 | FileCheck -check-prefix OVERFLOW-MSG %s
+; RUN: not FileCheck -check-prefix UNDERFLOW -input-file %s %s 2>&1 | FileCheck -check-prefix UNDERFLOW-MSG %s
 
 
 ; We ensure we attemt to match all lines with digits by using CHECK-NEXT
 ; directives for the checks to ensure each line of input is covered.
 
 
 ; Numeric variable definition with default matching format
 DEF DEFAULT FMT

 ; CHECK-LABEL: DEF DEFAULT FMT ALT SPC
 ; CHECK-NEXT: [[# VAR1a:]]
 ; CHECK-NEXT: [[# VAR1b :]]
 ; CHECK-NEXT: [[# VAR1c : ]]
 
 
 ; Numeric variable definition with explicit matching format
 DEF FMT
+-30
 c
 D
 ; CHECK-LABEL: DEF FMT
-; CHECK-NEXT: [[#%x,VAR2:]]
-; CHECK-NEXT: [[#%X,VAR3:]]
+; CHECK-NEXT: [[#%d,VAR2:]]
+; CHECK-NEXT: [[#%x,VAR3:]]
+; CHECK-NEXT: [[#%X,VAR4:]]
 
 
 ; Numeric variable definition with explicit matching format in alternate spacing
 DEF FMT ALT SPC
-c
-c
-c
-c
-c
+-30
+-30
+-30
+-30
+-30
 ; CHECK-LABEL: DEF FMT ALT SPC
-; CHECK-NEXT: [[#%x, VAR2a:]]
-; CHECK-NEXT: [[# %x, VAR2b:]]
-; CHECK-NEXT: [[# %x , VAR2c:]]
-; CHECK-NEXT: [[# %x , VAR2d :]]
-; CHECK-NEXT: [[# %x , VAR2e : ]]
+; CHECK-NEXT: [[#%d, VAR2a:]]
+; CHECK-NEXT: [[# %d, VAR2b:]]
+; CHECK-NEXT: [[# %d , VAR2c:]]
+; CHECK-NEXT: [[# %d , VAR2d :]]
+; CHECK-NEXT: [[# %d , VAR2e : ]]
 
 ; Numeric expressions in explicit matching format and default matching rule using
 ; variables defined on other lines
 USE DEF FMT IMPL MATCH
 11
 12
 10
+-30
+-29
+-31
 c
 d
 b
 1a
+1a
 D
 E
 C
 1B
+1B
 ; CHECK-LABEL: USE DEF FMT IMPL MATCH
 ; CHECK-NEXT: [[#%u,VAR1]]
 ; CHECK-NEXT: [[#%u,VAR1+1]]
 ; CHECK-NEXT: [[#%u,VAR1-1]]
-; CHECK-NEXT: [[#%x,VAR2]]
-; CHECK-NEXT: [[#%x,VAR2+1]]
-; CHECK-NEXT: [[#%x,VAR2-1]]
-; CHECK-NEXT: [[#%x,VAR2+14]]
-; CHECK-NEXT: [[#%X,VAR3]]
-; CHECK-NEXT: [[#%X,VAR3+1]]
-; CHECK-NEXT: [[#%X,VAR3-1]]
-; CHECK-NEXT: [[#%X,VAR3+14]]
+; CHECK-NEXT: [[#%d,VAR2]]
+; CHECK-NEXT: [[#%d,VAR2+1]]
+; CHECK-NEXT: [[#%d,VAR2-1]]
+; CHECK-NEXT: [[#%x,VAR3]]
+; CHECK-NEXT: [[#%x,VAR3+1]]
+; CHECK-NEXT: [[#%x,VAR3-1]]
+; CHECK-NEXT: [[#%x,VAR3+0xe]]
+; CHECK-NEXT: [[#%x,VAR3+0xE]]
+; CHECK-NEXT: [[#%X,VAR4]]
+; CHECK-NEXT: [[#%X,VAR4+1]]
+; CHECK-NEXT: [[#%X,VAR4-1]]
+; CHECK-NEXT: [[#%X,VAR4+0xe]]
+; CHECK-NEXT: [[#%X,VAR4+0xE]]
 
 
 ; Numeric expressions in explicit matching format and default matching rule using
 ; variables defined on other lines in alternate spacing
 USE EXPL FMT IMPL MATCH ALT SPC
 11
 11
 11

 
 
 ; Numeric expressions in implicit matching format and default matching rule using
 ; variables defined on other lines
 USE IMPL FMT IMPL MATCH
 11
 12
 10
+-30
+-29
+-31
 c
 d
 b
 1a
+1a
 D
 E
 C
 1B
+1B
 ; CHECK-LABEL: USE IMPL FMT IMPL MATCH
 ; CHECK-NEXT: [[#VAR1]]
 ; CHECK-NEXT: [[#VAR1+1]]
 ; CHECK-NEXT: [[#VAR1-1]]
 ; CHECK-NEXT: [[#VAR2]]
 ; CHECK-NEXT: [[#VAR2+1]]
 ; CHECK-NEXT: [[#VAR2-1]]
-; CHECK-NEXT: [[#VAR2+14]]
 ; CHECK-NEXT: [[#VAR3]]
 ; CHECK-NEXT: [[#VAR3+1]]
 ; CHECK-NEXT: [[#VAR3-1]]
-; CHECK-NEXT: [[#VAR3+14]]
+; CHECK-NEXT: [[#VAR3+0xe]]
+; CHECK-NEXT: [[#VAR3+0xE]]
+; CHECK-NEXT: [[#VAR4]]
+; CHECK-NEXT: [[#VAR4+1]]
+; CHECK-NEXT: [[#VAR4-1]]
+; CHECK-NEXT: [[#VAR4+0xe]]
+; CHECK-NEXT: [[#VAR4+0xE]]
 
 
 ; Explicit format override implicit format conflicts
 VAR USE IMPL OVERRIDE FMT CONFLICT
 23
 ; CHECK-LABEL: VAR USE IMPL OVERRIDE FMT CONFLICT
-; CHECK-NEXT: [[# %u, VAR1 + VAR2]]
+; CHECK-NEXT: [[# %u, VAR1 + VAR3]]
 
 
 ; Numeric expressions using variables defined on the command-line and an
 ; immediate interpreted as an unsigned value
 ; Note: 9223372036854775819 = 0x8000000000000000 + 11
 USE IMPL FMT IMPL MATCH UNSIGNED IMM
 9223372036854775819
 ; CHECK-LABEL: USE IMPL FMT IMPL MATCH UNSIGNED IMM
 ; CHECK-NEXT: [[#VAR1+0x8000000000000000]]
 
 
 ; Numeric expressions with conversion matching format and implicit matching rule
 ; using variables defined on other lines
 USE CONV FMT IMPL MATCH
 b
 B
 12
+12
+13
 13
 ; CHECK-LABEL: USE CONV FMT IMPL MATCH
 ; CHECK-NEXT: [[# %x, VAR1]]
 ; CHECK-NEXT: [[# %X, VAR1]]
-; CHECK-NEXT: [[# %u, VAR2]]
 ; CHECK-NEXT: [[# %u, VAR3]]
+; CHECK-NEXT: [[# %d, VAR3]]
+; CHECK-NEXT: [[# %u, VAR4]]
+; CHECK-NEXT: [[# %d, VAR4]]
 
 
 ; Numeric variable definition with unsupported matching format
 DEF INVALID FMT
 INVVAR1=a
 INVVAR2=11
 ; ERR-LABEL: DEF INVALID FMT
 ; INVALID-FMT-SPEC1-NEXT: INVVAR1=[[#%c,INVVAR1:]]

 ; Verify that when variable is set to an expression the expression is still
 ; checked
 DEF EXPR WRONG MATCH
 20
 43
 ; DEF-EXPR-FAIL-LABEL: DEF EXPR WRONG MATCH
 ; DEF-EXPR-FAIL-NEXT: [[# VAR20:]]
 ; DEF-EXPR-FAIL-NEXT: [[# VAR42: VAR20+22]]
 
 
 ; Conflicting implicit format
 VAR USE IMPL FMT CONFLICT
 23
 ; FMT-CONFLICT-LABEL: VAR USE IMPL FMT CONFLICT
 ; FMT-CONFLICT-NEXT: [[#VAR1 + VAR2]]
 ; FMT-CONFLICT-MSG: numeric-expression.txt:[[#@LINE-1]]:25: error: Variables with conflicting format specifier: need an explicit one
 ; FMT-CONFLICT-MSG-NEXT: {{F}}MT-CONFLICT-NEXT: {{\[\[#VAR1 \+ VAR2\]\]}}
 ; FMT-CONFLICT-MSG-NEXT: {{^                        \^$}}
+
+
+; Numeric expression with overflow
+OVERFLOW
+BIGVAR=10000000000000000
+; OVERFLOW-LABEL: OVERFLOW
+; OVERFLOW-NEXT: BIGVAR: [[#BIGVAR:0x8000000000000000+0x8000000000000000]]
+; OVERFLOW-MSG: numeric-expression.txt:[[#@LINE-1]]:29: 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
+UNDERFLOW
+TINYVAR=-10000000000000000
+; UNDERFLOW-LABEL: UNDERFLOW
+; UNDERFLOW-NEXT: TINYVAR: [[#%d,TINYVAR:-0x8000000000000000-0x8000000000000000]]
+; UNDERFLOW-MSG: numeric-expression.txt:[[#@LINE-1]]:31: error: Unable to substitute variable or numeric expression
+; UNDERFLOW-MSG-NEXT: {{U}}NDERFLOW-NEXT: TINYVAR: {{\[\[#%d,TINYVAR:-0x8000000000000000-0x8000000000000000\]\]}}
+; UNDERFLOW-MSG-NEXT: {{^                              \^$}}