[PATCH] D122227: Fix _BitInt suffix width calculation

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman created this revision.
aaron.ballman added reviewers: mgehre-amd, erichkeane.
Herald added a project: All.
aaron.ballman requested review of this revision.
Herald added a project: clang.

@mgehre-amd pointed out the following post-commit review feedback on the 
changes in 8cba72177dcd8de5d37177dbaf2347e5c1f0f1e8 
:

  As an example, the paper says 3wb /* Yields an _BitInt(3); two value bits, 
one sign bit */.
  So I would expect that 0xFwb gives _BitInt(5); four value bits, one sign bit, 
but with this implementation I get _BitInt(2).
  This is because ResultVal as 4 bits, and getMinSignedBits() inteprets it as 
negative and thus says that 1 bit is enough to represent -1.

This corrects the behavior for calculating the bit-width and adds some test 
coverage.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D17

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Lexer/bitint-constants.c


Index: clang/test/Lexer/bitint-constants.c
===
--- clang/test/Lexer/bitint-constants.c
+++ clang/test/Lexer/bitint-constants.c
@@ -142,3 +142,18 @@
   0x''''''''1wb; // expected-error {{integer 
literal is too large to be represented in any signed integer type}}
   0x''''''''1uwb; // expected-error {{integer 
literal is too large to be represented in any integer type}}
 }
+
+void TestTypes(void) {
+  // 2 value bits, one sign bit
+  _Static_assert(__builtin_types_compatible_p(__typeof__(3wb), _BitInt(3)));
+  // 2 value bits, one sign bit
+  _Static_assert(__builtin_types_compatible_p(__typeof__(-3wb), _BitInt(3)));
+  // 2 value bits, no sign bit
+  _Static_assert(__builtin_types_compatible_p(__typeof__(3uwb), unsigned 
_BitInt(2)));
+  // 4 value bits, one sign bit
+  _Static_assert(__builtin_types_compatible_p(__typeof__(0xFwb), _BitInt(5)));
+  // 4 value bits, one sign bit
+  _Static_assert(__builtin_types_compatible_p(__typeof__(-0xFwb), _BitInt(5)));
+  // 4 value bits, no sign bit
+  _Static_assert(__builtin_types_compatible_p(__typeof__(0xFuwb), unsigned 
_BitInt(4)));
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -3979,8 +3979,8 @@
   if (Literal.isBitInt) {
 // The signed version has one more bit for the sign value. There are no
 // zero-width bit-precise integers, even if the literal value is 0.
-Width = Literal.isUnsigned ? std::max(ResultVal.getActiveBits(), 1u)
-   : std::max(ResultVal.getMinSignedBits(), 
2u);
+Width = std::max(ResultVal.getActiveBits(), 1u) +
+(Literal.isUnsigned ? 0u : 1u);
 
 // Diagnose if the width of the constant is larger than 
BITINT_MAXWIDTH,
 // and reset the type to the largest supported width.


Index: clang/test/Lexer/bitint-constants.c
===
--- clang/test/Lexer/bitint-constants.c
+++ clang/test/Lexer/bitint-constants.c
@@ -142,3 +142,18 @@
   0x''''''''1wb; // expected-error {{integer literal is too large to be represented in any signed integer type}}
   0x''''''''1uwb; // expected-error {{integer literal is too large to be represented in any integer type}}
 }
+
+void TestTypes(void) {
+  // 2 value bits, one sign bit
+  _Static_assert(__builtin_types_compatible_p(__typeof__(3wb), _BitInt(3)));
+  // 2 value bits, one sign bit
+  _Static_assert(__builtin_types_compatible_p(__typeof__(-3wb), _BitInt(3)));
+  // 2 value bits, no sign bit
+  _Static_assert(__builtin_types_compatible_p(__typeof__(3uwb), unsigned _BitInt(2)));
+  // 4 value bits, one sign bit
+  _Static_assert(__builtin_types_compatible_p(__typeof__(0xFwb), _BitInt(5)));
+  // 4 value bits, one sign bit
+  _Static_assert(__builtin_types_compatible_p(__typeof__(-0xFwb), _BitInt(5)));
+  // 4 value bits, no sign bit
+  _Static_assert(__builtin_types_compatible_p(__typeof__(0xFuwb), unsigned _BitInt(4)));
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -3979,8 +3979,8 @@
   if (Literal.isBitInt) {
 // The signed version has one more bit for the sign value. There are no
 // zero-width bit-precise integers, even if the literal value is 0.
-Width = Literal.isUnsigned ? std::max(ResultVal.getActiveBits(), 1u)
-   : std::max(ResultVal.getMinSignedBits(), 2u);
+Width = std::max(ResultVal.getActiveBits(), 1u) +
+(Literal.isUnsigned ? 0u : 1u);
 
 // Diagnose if the width of the constant is larger than BITINT_MAXWIDTH,
 // and rese

[PATCH] D122227: Fix _BitInt suffix width calculation

[Matthias Gehre via Phabricator via cfe-commits]

mgehre-amd accepted this revision.
mgehre-amd added a comment.

Thanks a lot!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D17

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D122227: Fix _BitInt suffix width calculation

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman closed this revision.
aaron.ballman added a comment.

Thanks for the quick reviews, and thanks for catching the issue @mgehre-amd! 
I've committed in 9cf8f81ca45de198013f29442a7de6600b226d70 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D17

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits