Unbox struct locals and optimize compound ptr assignments to not copy the pointer

Around 40% speedup on Brunsli

Author: nunoplopes

cpp2rust/converter/models/boxed_vars.cpp

@@ -10,13 +10,19 @@
 namespace cpp2rust {
 namespace {
 // Whether decl may be stored without a Value, if its address is not taken:
-// scalars and arrays of non-arrays.
+// scalars, arrays of non-arrays, and user-defined structs, unless they have a
+// destructor, which is called with a pointer to them.
 bool CanUnbox(const clang::VarDecl *decl) {
   auto type = decl->getType();
   if (type->isConstantArrayType()) {
     if (type->getAsArrayTypeUnsafe()->getElementType()->isArrayType()) {
       return false;
     }
+  } else if (type->isStructureOrClassType()) {
+    if (!IsUserDefinedDecl(type->getAsRecordDecl()) ||
+        TypeNeedsDestruction(type)) {
+      return false;
+    }
   } else if (!type->isScalarType()) {
     return false;
   }
@@ -87,10 +93,17 @@ class AddressTakenVisitor
   }
 
 private:
-  // An element of an array is accessed without making a pointer to it.
+  // An element of an array, or a field of a struct, is accessed without
+  // making a pointer to it.
   void AddValueUse(clang::Expr *expr) {
     expr = expr->IgnoreParens();
-    if (auto *subscript = clang::dyn_cast<clang::ArraySubscriptExpr>(expr)) {
+    if (auto *member = clang::dyn_cast<clang::MemberExpr>(expr)) {
+      if (!member->isArrow() &&
+          clang::isa<clang::FieldDecl>(member->getMemberDecl())) {
+        AddValueUse(member->getBase());
+      }
+    } else if (auto *subscript =
+                   clang::dyn_cast<clang::ArraySubscriptExpr>(expr)) {
       auto *cast =
           clang::dyn_cast<clang::ImplicitCastExpr>(subscript->getBase());
       if (cast && cast->getCastKind() == clang::CK_ArrayToPointerDecay) {

cpp2rust/converter/models/boxed_vars.h

@@ -15,8 +15,8 @@ namespace cpp2rust {
 // to a function, or captured by a lambda. Any use that doesn't just read or
 // write the value of a variable is taken to make a pointer to it.
 //
-// Only scalars and arrays are stored directly; other variables are always
-// boxed.
+// Only scalars, arrays and user-defined structs are stored directly; other
+// variables are always boxed.
 class BoxedVars {
 public:
   explicit BoxedVars(clang::ASTContext &ctx);

cpp2rust/converter/models/converter_refcount.cpp

@@ -197,6 +197,18 @@ bool ConverterRefCount::IsUnboxedVar(const clang::ValueDecl *decl) const {
   return var && !boxed_vars_->contains(var);
 }
 
+bool ConverterRefCount::IsUnboxedPlace(const clang::Expr *expr) const {
+  expr = expr->IgnoreParenImpCasts();
+  // Fields of unions and references are accessed through pointers.
+  if (auto *member = clang::dyn_cast<clang::MemberExpr>(expr)) {
+    return !member->isArrow() && !member->getBase()->getType()->isUnionType() &&
+           !member->getMemberDecl()->getType()->isReferenceType() &&
+           IsUnboxedPlace(member->getBase());
+  }
+  auto *ref = clang::dyn_cast<clang::DeclRefExpr>(expr);
+  return ref && IsUnboxedVar(ref->getDecl());
+}
+
 ConverterRefCount::ConversionKind
 ConverterRefCount::VarConversionKind(const clang::VarDecl *decl) const {
   if (in_function_formals_) {
@@ -1914,7 +1926,7 @@ bool ConverterRefCount::VisitMemberExpr(clang::MemberExpr *expr) {
     return false;
   }
 
-  if (isAddrOf() && IsArrayFieldPtr(ctx_, expr)) {
+  if (isAddrOf() && IsArrayFieldPtr(ctx_, expr) && !IsUnboxedPlace(expr)) {
     StrCat(std::format("array_field_ptr!({}, {})", ConvertRecordPtr(expr),
                        GetNamedDeclAsString(member)));
     computed_expr_type_ = ComputedExprType::FreshPointer;
@@ -1980,6 +1992,13 @@ void ConverterRefCount::ConvertInlineField(clang::MemberExpr *expr) {
   if (isRValue() && !record_ptr_ &&
       (TypeIsCopyable(type) || type->isPointerType() || IsUniquePtr(type) ||
        expr == copied_expr_)) {
+    // A pointer in an unboxed struct is used in place, like a pointer
+    // variable, and only copied when needed.
+    if (type->isPointerType() && IsUnboxedPlace(expr)) {
+      StrCat(ReadField(expr, ""));
+      computed_expr_type_ = ComputedExprType::Pointer;
+      return;
+    }
     StrCat(ReadField(expr));
     SetFreshType(type);
     return;
@@ -2034,6 +2053,9 @@ std::string ConverterRefCount::ReadField(clang::MemberExpr *expr,
   if (!TypeIsCopyable(expr->getMemberDecl()->getType())) {
     str += copy;
   }
+  if (IsUnboxedPlace(expr)) {
+    return str;
+  }
   return ptr.empty() ? std::format("{{ {} }}", str)
                      : std::format("{}.with(|__s| {})", ptr, str);
 }
@@ -2567,10 +2589,20 @@ void ConverterRefCount::ConvertAssignment(clang::Expr *lhs, clang::Expr *rhs,
       auto ptr = pending_deref_.take();
       auto op = assign_operator;
       op.remove_suffix(1); // remove '='
-      {
-        PushBrace brace(*this);
+      PushBrace brace(*this);
+      if (lhs->getType()->isPointerType()) {
         StrCat(std::format("let _ptr = {}{};", ptr, fresh ? "" : ".clone()"));
         StrCat(std::format("_ptr.write(_ptr.read() {} {})", op, rhs_as_string));
+      } else {
+        // The place is accessed once, after the assigned value is computed,
+        // which may access it too.
+        if (rhs_as_string != "__rhs" && !rhs->isEvaluatable(ctx_)) {
+          StrCat(keyword::kLet, "__rhs", token::kAssign, rhs_as_string,
+                 token::kSemiColon);
+          rhs_as_string = "__rhs";
+        }
+        StrCat(std::format("{}.with_mut(|__v| *__v = *__v {} {})", ptr, op,
+                           rhs_as_string));
       }
     } else {
       StrCat(lhs_str, assign_operator, rhs_as_string);

cpp2rust/converter/models/converter_refcount.h

@@ -410,6 +410,9 @@ class ConverterRefCount final : public Converter {
   // Whether decl is a local variable or a parameter that is stored directly
   // instead of in a Value, as no pointer to it is ever made.
   bool IsUnboxedVar(const clang::ValueDecl *decl) const;
+  // Whether expr is an unboxed variable or a field of one, which is accessed
+  // in place.
+  bool IsUnboxedPlace(const clang::Expr *expr) const;
   std::shared_ptr<const BoxedVars> boxed_vars_;
 
   // Set when a constructor's translation refers to `this`, which then needs

libcc2rs-macros/src/record.rs

@@ -72,6 +72,7 @@ pub fn expand(input: TokenStream) -> TokenStream {
             const OFFSETS: #offsets = #offsets { #(#idents: #values,)* };
 
             #[allow(unused_comparisons)]
+            #[inline(always)]
             fn locate(
                 &self,
                 offset: usize,
@@ -85,6 +86,7 @@ pub fn expand(input: TokenStream) -> TokenStream {
             }
 
             #[allow(unused_comparisons)]
+            #[inline(always)]
             fn locate_mut(
                 &mut self,
                 offset: usize,

tests/multi-file/cross_tu_tag_collision/out/refcount/cross_tu_tag_collision.rs

@@ -13,9 +13,9 @@ pub struct widget {
     pub id: i32,
 }
 pub fn a_value_0() -> i32 {
-    let w: Value<widget> = <Value<widget>>::default();
-    (*w.borrow_mut()).id = 11;
-    return { (*w.borrow()).id };
+    let mut w: widget = <widget>::default();
+    w.id = 11;
+    return w.id;
 }
 pub fn main() {
     __cpp2rust_init_globals();

tests/multi-file/out_of_line_methods_cross_tu/out/refcount/out_of_line_methods_cross_tu.rs

@@ -77,8 +77,8 @@ impl SImpl for Ptr<S> {
     }
     fn add(&self, mut x: i32) -> i32 {
         {
-            let _ptr = field!((*self), v);
-            _ptr.write(_ptr.read() + x)
+            let __rhs = x;
+            field!((*self), v).with_mut(|__v| *__v = *__v + __rhs)
         };
         return (*self).with(|__s| __s.v);
     }

tests/ub/out/refcount/ctor_ref_member.rs

@@ -23,11 +23,11 @@ pub fn main() {
     std::process::exit(main_0());
 }
 fn main_0() -> i32 {
-    let s: Value<S> = Rc::new(RefCell::new({
+    let mut s: S = {
         let __tmp_0: Value<i32> = Rc::new(RefCell::new(5));
         S::new({ __tmp_0.as_pointer() })
-    }));
-    assert!((({ (*s.borrow()).r.clone() }.read()) == 5));
+    };
+    assert!((({ s.r.clone() }.read()) == 5));
     return 0;
 }
 pub fn __cpp2rust_init_globals() {}

tests/unit/out/refcount/10_struct.rs

@@ -61,26 +61,24 @@ pub fn main() {
     std::process::exit(main_0());
 }
 fn main_0() -> i32 {
-    let g: Value<Graph> = Rc::new(RefCell::new(Graph {
+    let mut g: Graph = Graph {
         V: 5_u32,
         adj: Ptr::<Ptr<GraphNode>>::null(),
-    }));
+    };
     let arr: Value<Box<[i32]>> = Rc::new(RefCell::new(Box::new([3, 1, 4])));
-    let it: Value<Partial> = Rc::new(RefCell::new(Partial::new_1({
-        (arr.as_pointer() as Ptr<i32>)
-    })));
-    if ({ { (*it.borrow()).p.clone() } } != { (arr.as_pointer() as Ptr<i32>) }) {
+    let mut it: Partial = Partial::new_1({ (arr.as_pointer() as Ptr<i32>) });
+    if ({ (it.p).clone() } != { (arr.as_pointer() as Ptr<i32>) }) {
         return 1;
     }
-    let def: Value<Partial> = Rc::new(RefCell::new(<Partial>::default()));
-    if !(({ (*def.borrow()).p.clone() }).is_null()) {
+    let mut def: Partial = <Partial>::default();
+    if !((def.p).is_null()) {
         return 1;
     }
-    let s: Value<S> = Rc::new(RefCell::new(S {
+    let mut s: S = S {
         i: 7,
         d: Ptr::<Declared>::null(),
-    }));
-    if ({ (*s.borrow()).i } != 7) || (!(({ (*s.borrow()).d.clone() }).is_null())) {
+    };
+    if (s.i != 7) || (!((s.d).is_null())) {
         return 1;
     }
     return 0;

tests/unit/out/refcount/anonymous-struct.rs

@@ -86,42 +86,42 @@ pub fn main() {
     std::process::exit(main_0());
 }
 fn main_0() -> i32 {
-    let o: Value<Outer> = Rc::new(RefCell::new(<Outer>::default()));
-    (*o.borrow_mut()).named.a = 1;
-    (*o.borrow_mut()).named.b = 2;
-    (*o.borrow_mut()).anonymous_named_0.c = 3;
-    (*o.borrow_mut()).anonymous_named_0.d = 4;
-    (*o.borrow_mut()).anonymous_named_1.g = 5;
-    (*o.borrow_mut()).anonymous_named_1.h = 6;
-    (*o.borrow_mut()).anon_2.e = 7;
-    (*o.borrow_mut()).anon_2.f = 8;
-    (*o.borrow_mut()).anon_3.i = 9;
-    (*o.borrow_mut()).anon_3.inner_named.j = 10;
-    (*o.borrow_mut()).anon_3.anon_5.k = 11;
-    assert!(({ (*o.borrow()).named.a } == 1));
-    assert!(({ (*o.borrow()).named.b } == 2));
-    assert!(({ (*o.borrow()).anonymous_named_0.c } == 3));
-    assert!(({ (*o.borrow()).anonymous_named_0.d } == 4));
-    assert!(({ (*o.borrow()).anonymous_named_1.g } == 5));
-    assert!(({ (*o.borrow()).anonymous_named_1.h } == 6));
-    assert!(({ (*o.borrow()).anon_2.e } == 7));
-    assert!(({ (*o.borrow()).anon_2.f } == 8));
-    assert!(({ (*o.borrow()).anon_3.i } == 9));
-    assert!(({ (*o.borrow()).anon_3.inner_named.j } == 10));
-    assert!(({ (*o.borrow()).anon_3.anon_5.k } == 11));
-    let s: Value<anon_6> = Rc::new(RefCell::new(<anon_6>::default()));
-    (*s.borrow_mut()).x = 1;
-    (*s.borrow_mut()).z = 2;
+    let mut o: Outer = <Outer>::default();
+    o.named.a = 1;
+    o.named.b = 2;
+    o.anonymous_named_0.c = 3;
+    o.anonymous_named_0.d = 4;
+    o.anonymous_named_1.g = 5;
+    o.anonymous_named_1.h = 6;
+    o.anon_2.e = 7;
+    o.anon_2.f = 8;
+    o.anon_3.i = 9;
+    o.anon_3.inner_named.j = 10;
+    o.anon_3.anon_5.k = 11;
+    assert!((o.named.a == 1));
+    assert!((o.named.b == 2));
+    assert!((o.anonymous_named_0.c == 3));
+    assert!((o.anonymous_named_0.d == 4));
+    assert!((o.anonymous_named_1.g == 5));
+    assert!((o.anonymous_named_1.h == 6));
+    assert!((o.anon_2.e == 7));
+    assert!((o.anon_2.f == 8));
+    assert!((o.anon_3.i == 9));
+    assert!((o.anon_3.inner_named.j == 10));
+    assert!((o.anon_3.anon_5.k == 11));
+    let mut s: anon_6 = <anon_6>::default();
+    s.x = 1;
+    s.z = 2;
     assert!(
         ({
-            (*s.borrow_mut()).x = 1;
-            { (*s.borrow()).x }
+            s.x = 1;
+            s.x
         } != 0)
     );
     assert!(
         ({
-            (*s.borrow_mut()).z = 2;
-            { (*s.borrow()).z }
+            s.z = 2;
+            s.z
         } != 0)
     );
     return 0;