From fd689f5fdd0c69c0fb3f03c0fb97b2d5b926ad7a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Uwe=20Kr=C3=BCger?=
 <45282134+UweKrueger@users.noreply.github.com>
Date: Mon, 7 Dec 2020 17:13:19 +0100
Subject: [PATCH] cgen: optimize array op assign (#7178)

---
 vlib/builtin/array_test.v | 51 ++++++++++++++++++++++++++
 vlib/v/gen/cgen.v         | 76 ++++++++++++++-------------------------
 2 files changed, 77 insertions(+), 50 deletions(-)

diff --git a/vlib/builtin/array_test.v b/vlib/builtin/array_test.v
index af3e258988..07cb3e2ebb 100644
--- a/vlib/builtin/array_test.v
+++ b/vlib/builtin/array_test.v
@@ -796,6 +796,57 @@ fn test_in_struct() {
 	assert baz.bar[0] == 3
 }
 
+[direct_array_access]
+fn test_direct_modification() {
+	mut foo := [2, 0, 5]
+	foo[1] = 3
+	foo[0] *= 7
+	foo[1]--
+	foo[2] -= 2
+	assert foo[0] == 14
+	assert foo[1] == 2
+	assert foo[2] == 3
+}
+
+fn test_shared_modification() {
+	shared foo := &[2, 0, 5]
+	lock foo {
+		unsafe {
+			foo[1] = 3
+			foo[0] *= 7
+			foo[1]--
+			foo[2] -= 2
+		}
+	}
+	rlock foo {
+		unsafe {
+			assert foo[0] == 14
+			assert foo[1] == 2
+			assert foo[2] == 3
+		}
+	}
+}
+
+[direct_array_access]
+fn test_shared_direct_modification() {
+	shared foo := &[2, 0, 5]
+	lock foo {
+		unsafe {
+			foo[1] = 3
+			foo[0] *= 7
+			foo[1]--
+			foo[2] -= 2
+		}
+	}
+	rlock foo {
+		unsafe {
+			assert foo[0] == 14
+			assert foo[1] == 2
+			assert foo[2] == 3
+		}
+	}
+}
+
 fn test_bools() {
 	println('test b')
 	mut a := [true, false]
diff --git a/vlib/v/gen/cgen.v b/vlib/v/gen/cgen.v
index a18645ca0d..b433a77a90 100644
--- a/vlib/v/gen/cgen.v
+++ b/vlib/v/gen/cgen.v
@@ -3666,12 +3666,18 @@ fn (mut g Gen) index_expr(node ast.IndexExpr) {
 				is_selector := node.left is ast.SelectorExpr
 				if g.is_assign_lhs && !is_selector && node.is_setter {
 					is_direct_array_access := g.fn_decl != 0 && g.fn_decl.is_direct_arr
+					is_op_assign := g.assign_op != .assign && info.elem_type != table.string_type
 					array_ptr_type_str := match elem_typ.kind {
 						.function { 'voidptr*' }
 						else { '$elem_type_str*' }
 					}
 					if is_direct_array_access {
 						g.write('(($array_ptr_type_str)')
+					} else if is_op_assign {
+						g.write('(*($array_ptr_type_str)array_get(')
+						if left_is_ptr && !node.left_type.has_flag(.shared_f) {
+							g.write('*')
+						}
 					} else {
 						g.is_array_set = true // special handling of assign_op and closing with '})'
 						g.write('array_set(')
@@ -3680,7 +3686,6 @@ fn (mut g Gen) index_expr(node ast.IndexExpr) {
 						}
 					}
 					g.expr(node.left)
-					// TODO: test direct_array_access when 'shared' is implemented
 					if node.left_type.has_flag(.shared_f) {
 						if left_is_ptr {
 							g.write('->val')
@@ -3700,59 +3705,30 @@ fn (mut g Gen) index_expr(node ast.IndexExpr) {
 					} else {
 						g.write(', ')
 						g.expr(node.index)
-						mut need_wrapper := true
-						/*
-						match node.right {
-							ast.EnumVal, ast.Ident {
-								// `&x` is enough for variables and enums
-								// `&(Foo[]){ ... }` is only needed for function calls and literals
-								need_wrapper = false
+						if !is_op_assign {
+							mut need_wrapper := true
+							/*
+							match node.right {
+								ast.EnumVal, ast.Ident {
+									// `&x` is enough for variables and enums
+									// `&(Foo[]){ ... }` is only needed for function calls and literals
+									need_wrapper = false
+								}
+								else {}
 							}
-							else {}
-						}
-						*/
-						if need_wrapper {
-							if elem_typ.kind == .function {
-								g.write(', &(voidptr[]) { ')
+							*/
+							if need_wrapper {
+								if elem_typ.kind == .function {
+									g.write(', &(voidptr[]) { ')
+								} else {
+									g.write(', &($elem_type_str[]) { ')
+								}
 							} else {
-								g.write(', &($elem_type_str[]) { ')
+								g.write(', &')
 							}
 						} else {
-							g.write(', &')
-						}
-						// `x[0] *= y`
-						if g.assign_op != .assign &&
-							g.assign_op in token.assign_tokens && info.elem_type != table.string_type {
-							// TODO move this
-							g.write('*($elem_type_str*)array_get(')
-							if left_is_ptr && !node.left_type.has_flag(.shared_f) {
-								g.write('*')
-							}
-							g.expr(node.left)
-							if node.left_type.has_flag(.shared_f) {
-								if left_is_ptr {
-									g.write('->val')
-								} else {
-									g.write('.val')
-								}
-							}
-							g.write(', ')
-							g.expr(node.index)
-							g.write(') ')
-							op := match g.assign_op {
-								.mult_assign { '*' }
-								.plus_assign { '+' }
-								.minus_assign { '-' }
-								.div_assign { '/' }
-								.xor_assign { '^' }
-								.mod_assign { '%' }
-								.or_assign { '|' }
-								.and_assign { '&' }
-								.left_shift_assign { '<<' }
-								.right_shift_assign { '>>' }
-								else { '' }
-							}
-							g.write(op)
+							// `x[0] *= y`
+							g.write('))')
 						}
 					}
 				} else {