[PATCH] D81420: Fix size for _ExtInt types with builtins

[Mott, Jeffrey T via Phabricator via cfe-commits]

jtmott-intel created this revision.
jtmott-intel added reviewers: erichkeane, rjmccall.

This patch addresses two issues.

The first issue is use of the checked arithmetic builtins <https://clang.llvm.org/docs/LanguageExtensions.html#checked-arithmetic-builtins> in combination with _ExtInt <http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2472.pdf> types. _ExtInt types with non-power of two sizes would be (inconsistently) rounded up to the next largest power of two. The inconsistency caused the following compile error:

  $ cat test.c
  int main(void) {
      _ExtInt(31) x = 1;
      _ExtInt(31) y = 1;
      _ExtInt(31) result;
      _Bool status = __builtin_add_overflow(x, y, &result);
      return 0;
  }
  
  $ clang test.c
  clang-11: /home/user/llvm_workspace/llvm-project/llvm/lib/IR/Instructions.cpp:1408: void llvm::StoreInst::AssertOK(): Assertion `getOperand(0)->getType() == cast<PointerType>(getOperand(1)->getType())->getElementType() && "Ptr must be a pointer to Val type!"' failed.

To fix this, I added the above example as a failing test to `clang/test/CodeGen/builtins-overflow.c`, and I changed `clang/lib/CodeGen/CGBuiltin.cpp` to get the correct size for _ExtInt types and allow the new test to pass.

The second issue is with `__builtin_mul_overflow` specifically in combination with _ExtInt types larger than 64 bits. For that combination, the emitted IR appears to be correct, but a full compile caused the following link error (for at least X86 target):

  $ cat test.c
  int main(void) {
      _ExtInt(65) x = 1;
      _ExtInt(65) y = 1;
      _ExtInt(65) result;
      _Bool status = __builtin_mul_overflow(x, y, &result);
      return 0;
  }
  $ clang test.c
  /tmp/test-2b8174.o: In function `main':
  test.c:(.text+0x63): undefined reference to `__muloti4'
  clang-11: error: linker command failed with exit code 1 (use -v to see invocation)

To address this -- for now -- I changed `clang/lib/Sema/SemaChecking.cpp` to emit a diagnostic that explains the limitation to users better than the link error does. I did my best to borrow verbiage from similar messages. The new compile result is:

  $ clang test.c
  test.c:5:43: error: _ExtInt argument larger than 64-bits to overflow builtin requires runtime support that is not available for this target
      _Bool status = __builtin_mul_overflow(x, y, &result);
                                            ^
  1 error generated.


https://reviews.llvm.org/D81420

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/builtins-overflow.c

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D81420.269306.patch
Type: text/x-patch
Size: 5587 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200608/be00190e/attachment-0001.bin>