[PATCH] D80915: [FileCheck] Implement * and / operators for ExpressionValue.

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jun 9 04:21:19 PDT 2020


jhenderson added inline comments.


================
Comment at: llvm/unittests/Support/FileCheckTest.cpp:518
+  expectOperationValueResult(operator/, MinInt64, 1, MinInt64);
+  expectOperationValueResult(operator/, static_cast<uint64_t>(MaxInt64)+1, -1,
+                             MinInt64);
----------------
paulwalker-arm wrote:
> jhenderson wrote:
> > Same as above - 2's complement assumption.
> I'm not sure what to do here.  With the current design of ExpressionValue the overflow scenarios change depending on whether you're 1s or 2s complement.  I started going down the path of making overflow reporting consistent (essentially making ExpressionValue a 65bit 1s complement number) but I'm worried this might not be what you want.
> 
> I could just not have tests for the cases where 1s complement differs from 2s complement, but that seems wrong, hence the request for guidance.
Sorry for the confusion. I don't think we should change the implementation to enable consistent testing depending on the underlying numeric representation. It's probably best to simply not bother testing the edge cases so precisely, since we can't control it sensibly.

My suggestion would be to allow a bit of leeway. For example, you could show that `MaxInt64 \ -1` is equal to `-MaxInt64`. That should be representable under the permitted numeric representations.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80915/new/

https://reviews.llvm.org/D80915





More information about the llvm-commits mailing list