[clang] [clang][Interp] Implement compound assign operators on bitfields (PR #67306)

Timm Baeder via cfe-commits cfe-commits at lists.llvm.org
Mon Sep 25 02:40:41 PDT 2023


Timm =?utf-8?q?Bäder?= <tbaeder at redhat.com>,
Timm =?utf-8?q?Bäder?= <tbaeder at redhat.com>
Message-ID: <llvm/llvm-project/pull/67306/clang at github.com>
In-Reply-To:


https://github.com/tbaederr created https://github.com/llvm/llvm-project/pull/67306

None

>From 2cd32e94dd4eb194fd70b481f50eeaa2b6f72b66 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbaeder at redhat.com>
Date: Wed, 13 Sep 2023 14:11:00 +0200
Subject: [PATCH 1/3] [clang] Avoid evaluating the BitWidth expression over and
 over again

It's usually a ConstantExpr with a saved constant integer anyway, so we
can just return that.
---
 clang/lib/AST/Decl.cpp | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index 08ae2087cfe70eb..e283f921905b4eb 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -4471,6 +4471,14 @@ void FieldDecl::setLazyInClassInitializer(LazyDeclStmtPtr NewInit) {
 
 unsigned FieldDecl::getBitWidthValue(const ASTContext &Ctx) const {
   assert(isBitField() && "not a bitfield");
+
+  // Try to avoid evaluating the expression in the overwhelmingly
+  // common case that we have a ConstantExpr.
+  if (const auto *CE = dyn_cast<ConstantExpr>(getBitWidth())) {
+    assert(CE->hasAPValueResult());
+    return CE->getResultAsAPSInt().getZExtValue();
+  }
+
   return getBitWidth()->EvaluateKnownConstInt(Ctx).getZExtValue();
 }
 

>From 7c502534c4062d6f2126b57d105cfde1b72f7b67 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbaeder at redhat.com>
Date: Thu, 10 Aug 2023 09:19:05 +0200
Subject: [PATCH 2/3] [clang][Interp] Basic support for bit fields

Differential Revision: https://reviews.llvm.org/D155270
---
 clang/lib/AST/Interp/ByteCodeExprGen.cpp | 17 +++++++--
 clang/lib/AST/Interp/ByteCodeStmtGen.cpp |  9 ++++-
 clang/lib/AST/Interp/Interp.h            | 14 +++----
 clang/lib/AST/Interp/Record.h            |  1 +
 clang/test/AST/Interp/bitfields.cpp      | 47 ++++++++++++++++++++++++
 5 files changed, 75 insertions(+), 13 deletions(-)
 create mode 100644 clang/test/AST/Interp/bitfields.cpp

diff --git a/clang/lib/AST/Interp/ByteCodeExprGen.cpp b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
index e813d4fa651ceaf..cb3cfd835ad01df 100644
--- a/clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -316,8 +316,10 @@ bool ByteCodeExprGen<Emitter>::VisitBinaryOperator(const BinaryOperator *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:
@@ -521,8 +523,15 @@ bool ByteCodeExprGen<Emitter>::visitInitList(ArrayRef<const Expr *> Inits,
       const Record::Field *FieldToInit = R->getField(InitIndex);
       if (!this->visit(Init))
         return false;
-      if (!this->emitInitField(*T, FieldToInit->Offset, E))
-        return false;
+
+      if (FieldToInit->isBitField()) {
+        if (!this->emitInitBitField(*T, FieldToInit, E))
+          return false;
+      } else {
+        if (!this->emitInitField(*T, FieldToInit->Offset, E))
+          return false;
+      }
+
       if (!this->emitPopPtr(E))
         return false;
       ++InitIndex;
diff --git a/clang/lib/AST/Interp/ByteCodeStmtGen.cpp b/clang/lib/AST/Interp/ByteCodeStmtGen.cpp
index 15eae8e20b3a678..b57bf3476d931a9 100644
--- a/clang/lib/AST/Interp/ByteCodeStmtGen.cpp
+++ b/clang/lib/AST/Interp/ByteCodeStmtGen.cpp
@@ -166,8 +166,13 @@ bool ByteCodeStmtGen<Emitter>::visitFunc(const FunctionDecl *F) {
           if (!this->visit(InitExpr))
             return false;
 
-          if (!this->emitInitThisField(*T, F->Offset, 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.
diff --git a/clang/lib/AST/Interp/Interp.h b/clang/lib/AST/Interp/Interp.h
index 8453856e526a6b2..f9cfa3bd00bf67a 100644
--- a/clang/lib/AST/Interp/Interp.h
+++ b/clang/lib/AST/Interp/Interp.h
@@ -1037,6 +1037,7 @@ bool InitThisField(InterpState &S, CodePtr OpPC, uint32_t I) {
 
 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();
@@ -1078,8 +1079,9 @@ bool InitField(InterpState &S, CodePtr OpPC, uint32_t I) {
 
 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();
@@ -1294,11 +1296,10 @@ bool StoreBitField(InterpState &S, CodePtr OpPC) {
     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;
 }
 
@@ -1310,11 +1311,10 @@ bool StoreBitFieldPop(InterpState &S, CodePtr OpPC) {
     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;
 }
 
diff --git a/clang/lib/AST/Interp/Record.h b/clang/lib/AST/Interp/Record.h
index b81070aa77e8413..b0952af2d1ac6cc 100644
--- a/clang/lib/AST/Interp/Record.h
+++ b/clang/lib/AST/Interp/Record.h
@@ -29,6 +29,7 @@ class Record final {
     const FieldDecl *Decl;
     unsigned Offset;
     Descriptor *Desc;
+    bool isBitField() const { return Decl->isBitField(); }
   };
 
   /// Describes a base class.
diff --git a/clang/test/AST/Interp/bitfields.cpp b/clang/test/AST/Interp/bitfields.cpp
new file mode 100644
index 000000000000000..e078704fce51ff0
--- /dev/null
+++ b/clang/test/AST/Interp/bitfields.cpp
@@ -0,0 +1,47 @@
+// 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, "");
+}

>From 592e49002f739d1dea5f9c007f6ad64c0f25b54b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbaeder at redhat.com>
Date: Mon, 25 Sep 2023 11:39:02 +0200
Subject: [PATCH 3/3] [clang][Interp] Implement compound assign operators on
 bitfields

---
 clang/lib/AST/Interp/ByteCodeExprGen.cpp |  7 ++++-
 clang/test/AST/Interp/bitfields.cpp      | 38 ++++++++++++++++++++++--
 2 files changed, 42 insertions(+), 3 deletions(-)

diff --git a/clang/lib/AST/Interp/ByteCodeExprGen.cpp b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
index cb3cfd835ad01df..025571906528d03 100644
--- a/clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -1115,8 +1115,13 @@ bool ByteCodeExprGen<Emitter>::VisitCompoundAssignOperator(
   }
 
   // And store the result in LHS.
-  if (DiscardResult)
+  if (DiscardResult) {
+    if (LHS->refersToBitField())
+      return this->emitStoreBitFieldPop(*ResultT, E);
     return this->emitStorePop(*ResultT, E);
+  }
+  if (LHS->refersToBitField())
+    return this->emitStoreBitField(*ResultT, E);
   return this->emitStore(*ResultT, E);
 }
 
diff --git a/clang/test/AST/Interp/bitfields.cpp b/clang/test/AST/Interp/bitfields.cpp
index e078704fce51ff0..9a144e2f0d9610e 100644
--- a/clang/test/AST/Interp/bitfields.cpp
+++ b/clang/test/AST/Interp/bitfields.cpp
@@ -31,8 +31,6 @@ namespace Basic {
     return a.a = 10;
   }
   static_assert(storeA2() == 2, "");
-
-  // TODO: +=, -=, etc. operators.
 }
 
 namespace Overflow {
@@ -45,3 +43,39 @@ namespace Overflow {
 
   static_assert(f() == 3, "");
 }
+
+namespace Compound {
+  struct A {
+    unsigned int a : 2;
+    constexpr A() : a(0) {}
+    constexpr A(int a) : a(a) {}
+  };
+
+  constexpr unsigned add() {
+    A a;
+    a.a += 10;
+    return a.a;
+  }
+  static_assert(add() == 2, "");
+
+  constexpr unsigned sub() {
+    A a;
+    a.a -= 10;
+    return a.a;
+  }
+  static_assert(sub() == 2, "");
+
+  constexpr unsigned mul() {
+    A a(1);
+    a.a *= 5;
+    return a.a;
+  }
+  static_assert(mul() == 1, "");
+
+  constexpr unsigned div() {
+    A a(2);
+    a.a /= 2;
+    return a.a;
+  }
+  static_assert(div() == 1, "");
+}



More information about the cfe-commits mailing list