[clang][Interp] Basic support for bit fields

[shafik]

clang/test/AST/Interp/bitfields.cpp

37

This is an interesting test case:

struct A {int c:3;};

constexpr int f() {
  A a1{3};
  return a1.c++;
}

void g() {
    constexpr int x = f();
}

We are overflowing the bit-field value but it is implementation defined what the value is 😱 CC @aaron.ballman

Might be worth checking the result of conditional operator and comma etc are bit-fields when they are supposed to be and therefore sizeof fails for those cases.

[tbaeder]

Ping

[tbaeder]

Ping

[aaron.ballman]

Precommit CI is currently crashing on the newly introduced test case.

clang/test/AST/Interp/bitfields.cpp

37

Yeah, the bit-field overflow case is http://eel.is/c++draft/expr.ass#4 -- we don't document what we do in this case (so this is a good opportunity to improve our docs), but we appear to treat it the same as overflow for a non-bit-field integer type: https://godbolt.org/z/4Ta7KoP58

[tbaeder]

In D155270#4550466, @aaron.ballman wrote:

Precommit CI is currently crashing on the newly introduced test case.

It's missing https://reviews.llvm.org/D155548 :)

That's of course just a workaround. At the beginning of the evalute* fuctions in the new interpreter, there's a assert(S.Stk.empty()), to ensure that a previous interpret call didn't leave someting on the stack. But that doesn't work anymore since FieldDecl::getBitWidthValue() calls in the interpreter again while we're already evaluating something, so that assertion triggers.

[aaron.ballman]

In D155270#4550473, @tbaeder wrote:

In D155270#4550466, @aaron.ballman wrote:

Precommit CI is currently crashing on the newly introduced test case.

It's missing https://reviews.llvm.org/D155548 :)

That's of course just a workaround. At the beginning of the evalute* fuctions in the new interpreter, there's a assert(S.Stk.empty()), to ensure that a previous interpret call didn't leave someting on the stack. But that doesn't work anymore since FieldDecl::getBitWidthValue() calls in the interpreter again while we're already evaluating something, so that assertion triggers.

Ah, I see, thank you for the explanation!

There's more work left here for supporting other operators and uses, but as far as these changes go, they seem reasonable to me. It'd be good to add Shafik's suggested test cases even if they're surrounded with FIXME comments, just so we don't lose track of the request.

clang/lib/AST/Interp/ByteCodeExprGen.cpp

bool ByteCodeExprGen<Emitter>::VisitBinaryOperator(const BinaryOperator *BO) {
   case BO_Rem:
     return Discard(this->emitRem(*T, BO));
   case BO_Div:
     if (BO->getType()->isFloatingType())
       return Discard(this->emitDivf(getRoundingMode(BO), BO));
     return Discard(this->emitDiv(*T, BO));
   case BO_Assign:
     if (DiscardResult)
-      return this->emitStorePop(*T, BO);
-    return this->emitStore(*T, BO);
+      return LHS->refersToBitField() ? this->emitStoreBitFieldPop(*T, BO)
+                                     : this->emitStorePop(*T, BO);
+    return LHS->refersToBitField() ? this->emitStoreBitField(*T, BO)
+                                   : this->emitStore(*T, BO);
   case BO_And:
     return Discard(this->emitBitAnd(*T, BO));
   case BO_Or:
     return Discard(this->emitBitOr(*T, BO));
   case BO_Shl:
     return Discard(this->emitShl(*LT, *RT, BO));
   case BO_Shr:
     return Discard(this->emitShr(*LT, *RT, BO));
for (const Expr *Init : InitList->inits()) {
       if (!this->emitDupPtr(Initializer))
         return false;
 
       if (std::optional<PrimType> T = classify(Init)) {
         const Record::Field *FieldToInit = R->getField(InitIndex);
         if (!this->visit(Init))
           return false;
 
+        if (FieldToInit->isBitField()) {
+          if (!this->emitInitBitField(*T, FieldToInit, Initializer))
+            return false;
+        } else {
           if (!this->emitInitField(*T, FieldToInit->Offset, Initializer))
             return false;
+        }
 
         if (!this->emitPopPtr(Initializer))
           return false;
         ++InitIndex;
       } else {
         // Initializer for a direct base class.
         if (const Record::Base *B = R->getBase(Init->getType())) {
           if (!this->emitGetPtrBasePop(B->Offset, Init))

clang/lib/AST/Interp/ByteCodeStmtGen.cpp

for (const auto *Init : Ctor->inits()) {
       const Expr *InitExpr = Init->getInit();
       if (const FieldDecl *Member = Init->getMember()) {
         const Record::Field *F = R->getField(Member);
 
         if (std::optional<PrimType> T = this->classify(InitExpr)) {
           if (!this->visit(InitExpr))
             return false;
 
+          if (F->isBitField()) {
+            if (!this->emitInitThisBitField(*T, F, InitExpr))
+              return false;
+          } else {
             if (!this->emitInitThisField(*T, F->Offset, InitExpr))
               return false;
+          }
         } else {
           // Non-primitive case. Get a pointer to the field-to-initialize
           // on the stack and call visitInitialzer() for it.
           if (!this->emitThis(InitExpr))
             return false;
 
           if (!this->emitGetPtrField(F->Offset, InitExpr))
             return false;

clang/lib/AST/Interp/Interp.h

bool InitThisField(InterpState &S, CodePtr OpPC, uint32_t I) {
   const Pointer &Field = This.atField(I);
   Field.deref<T>() = S.Stk.pop<T>();
   Field.initialize();
   return true;
 }
 
 template <PrimType Name, class T = typename PrimConv<Name>::T>
 bool InitThisBitField(InterpState &S, CodePtr OpPC, const Record::Field *F) {
+  assert(F->isBitField());
   if (S.checkingPotentialConstantExpression())
     return false;
   const Pointer &This = S.Current->getThis();
   if (!CheckThis(S, OpPC, This))
     return false;
   const Pointer &Field = This.atField(F->Offset);
   const auto &Value = S.Stk.pop<T>();
   Field.deref<T>() = Value.truncate(F->Decl->getBitWidthValue(S.getCtx()));
bool InitField(InterpState &S, CodePtr OpPC, uint32_t I) {
   Field.deref<T>() = Value;
   Field.activate();
   Field.initialize();
   return true;
 }
 
 template <PrimType Name, class T = typename PrimConv<Name>::T>
 bool InitBitField(InterpState &S, CodePtr OpPC, const Record::Field *F) {
+  assert(F->isBitField());
   const T &Value = S.Stk.pop<T>();
-  const Pointer &Field = S.Stk.pop<Pointer>().atField(F->Offset);
+  const Pointer &Field = S.Stk.peek<Pointer>().atField(F->Offset);
   Field.deref<T>() = Value.truncate(F->Decl->getBitWidthValue(S.getCtx()));
   Field.activate();
   Field.initialize();
   return true;
 }
 
 template <PrimType Name, class T = typename PrimConv<Name>::T>
 bool InitFieldActive(InterpState &S, CodePtr OpPC, uint32_t I) {

 template <PrimType Name, class T = typename PrimConv<Name>::T>
 bool StoreBitField(InterpState &S, CodePtr OpPC) {
   const T &Value = S.Stk.pop<T>();
   const Pointer &Ptr = S.Stk.peek<Pointer>();
   if (!CheckStore(S, OpPC, Ptr))
     return false;
   if (!Ptr.isRoot())
     Ptr.initialize();
-  if (auto *FD = Ptr.getField()) {
+  if (const auto *FD = Ptr.getField())
     Ptr.deref<T>() = Value.truncate(FD->getBitWidthValue(S.getCtx()));
-  } else {
+  else
     Ptr.deref<T>() = Value;
-  }
   return true;
 }
 
 template <PrimType Name, class T = typename PrimConv<Name>::T>
 bool StoreBitFieldPop(InterpState &S, CodePtr OpPC) {
   const T &Value = S.Stk.pop<T>();
   const Pointer &Ptr = S.Stk.pop<Pointer>();
   if (!CheckStore(S, OpPC, Ptr))
     return false;
   if (!Ptr.isRoot())
     Ptr.initialize();
-  if (auto *FD = Ptr.getField()) {
+  if (const auto *FD = Ptr.getField())
     Ptr.deref<T>() = Value.truncate(FD->getBitWidthValue(S.getCtx()));
-  } else {
+  else
     Ptr.deref<T>() = Value;
-  }
   return true;
 }
 
 template <PrimType Name, class T = typename PrimConv<Name>::T>
 bool InitPop(InterpState &S, CodePtr OpPC) {
   const T &Value = S.Stk.pop<T>();
   const Pointer &Ptr = S.Stk.pop<Pointer>();
   if (!CheckInit(S, OpPC, Ptr))

clang/lib/AST/Interp/Record.h

 /// Structure/Class descriptor.
 class Record final {
 public:
   /// Describes a record field.
   struct Field {
     const FieldDecl *Decl;
     unsigned Offset;
     Descriptor *Desc;
+    bool isBitField() const { return Decl->isBitField(); }
   };
 
   /// Describes a base class.
   struct Base {
     const RecordDecl *Decl;
     unsigned Offset;
     Descriptor *Desc;
     Record *R;

clang/test/AST/Interp/bitfields.cpp

+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -Wno-bitfield-constant-conversion -verify %s
+// RUN: %clang_cc1 -verify=ref -Wno-bitfield-constant-conversion %s
+
+// expected-no-diagnostics
+// ref-no-diagnostics
+
+namespace Basic {
+  struct A {
+    unsigned int a : 2;
+    constexpr A() : a(0) {}
+    constexpr A(int a) : a(a) {}
+  };
+
+  constexpr A a{1};
+  static_assert(a.a == 1, "");
+
+  constexpr A a2{10};
+  static_assert(a2.a == 2, "");
+
+
+  constexpr int storeA() {
+    A a;
+    a.a = 10;
+
+    return a.a;
+  }
+  static_assert(storeA() == 2, "");
+
+  constexpr int storeA2() {
+    A a;
+    return a.a = 10;
+  }
+  static_assert(storeA2() == 2, "");
+
+  // TODO: +=, -=, etc. operators.
+}
+
+namespace Overflow {
+  struct A {int c:3;};
+
+  constexpr int f() {
+    A a1{3};
+    return a1.c++;
+  }
+
+  static_assert(f() == 3, "");
+}