[mlir][emitc] Relax hasSideEffect rules for Var/Member
#145011
Conversation
|
@llvm/pr-subscribers-mlir-emitc Author: Kirill Chibisov (kchibisov) ChangesThe load side effect semantic depends on the operation that was used to declare the load's operand. In case of -- Not sure whether the logic of #144990 should be applied here, since the Full diff: https://github.com/llvm/llvm-project/pull/145011.diff 4 Files Affected:
diff --git a/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td b/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td
index 9ecdb74f4d82e..d88407a4d6726 100644
--- a/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td
+++ b/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td
@@ -966,6 +966,10 @@ def EmitC_LoadOp : EmitC_Op<"load", [CExpressionInterface,
let results = (outs AnyType:$result);
let assemblyFormat = "$operand attr-dict `:` type($operand)";
+
+ let extraClassDeclaration = [{
+ bool hasSideEffects();
+ }];
}
def EmitC_MulOp : EmitC_BinaryOp<"mul", []> {
diff --git a/mlir/lib/Dialect/EmitC/IR/EmitC.cpp b/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
index e602210c2dc6c..10e0d931e17fd 100644
--- a/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
+++ b/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
@@ -898,6 +898,20 @@ LogicalResult emitc::VariableOp::verify() {
return verifyInitializationAttribute(getOperation(), getValueAttr());
}
+//===----------------------------------------------------------------------===//
+// VariableOp
+//===----------------------------------------------------------------------===//
+
+bool emitc::LoadOp::hasSideEffects() {
+ auto *defOp = this->getOperand().getDefiningOp();
+ // The load side effect semantic depends on the operation that was used to
+ // declare the load's operand, `VariableOp` and `MemberOp` perform loading
+ // from the stack, thus we may consider them as not having side effect to
+ // make end code more readable by letting those loads to get inlined.
+ return !defOp ||
+ !(llvm::isa<VariableOp>(defOp) || llvm::isa<MemberOp>(defOp));
+}
+
//===----------------------------------------------------------------------===//
// YieldOp
//===----------------------------------------------------------------------===//
diff --git a/mlir/test/Dialect/EmitC/transforms.mlir b/mlir/test/Dialect/EmitC/transforms.mlir
index a38f396dad953..687ebe2cd3612 100644
--- a/mlir/test/Dialect/EmitC/transforms.mlir
+++ b/mlir/test/Dialect/EmitC/transforms.mlir
@@ -135,21 +135,18 @@ func.func @single_result_requirement() -> (i32, i32) {
// CHECK-SAME: %[[VAL_1:.*]]: !emitc.ptr<i32>) -> i1 {
// CHECK: %[[VAL_2:.*]] = "emitc.constant"() <{value = 0 : i64}> : () -> i64
// CHECK: %[[VAL_3:.*]] = "emitc.variable"() <{value = #emitc.opaque<"42">}> : () -> !emitc.lvalue<i32>
-// CHECK: %[[VAL_4:.*]] = emitc.expression : i32 {
-// CHECK: %[[VAL_5:.*]] = load %[[VAL_3]] : <i32>
-// CHECK: yield %[[VAL_5]] : i32
+// CHECK: %[[VAL_4:.*]] = emitc.subscript %[[VAL_1]]{{\[}}%[[VAL_2]]] : (!emitc.ptr<i32>, i64) -> !emitc.lvalue<i32>
+// CHECK: %[[VAL_5:.*]] = emitc.expression : i32 {
+// CHECK: %[[VAL_6:.*]] = load %[[VAL_4]] : <i32>
+// CHECK: yield %[[VAL_6]] : i32
// CHECK: }
-// CHECK: %[[VAL_6:.*]] = emitc.subscript %[[VAL_1]]{{\[}}%[[VAL_2]]] : (!emitc.ptr<i32>, i64) -> !emitc.lvalue<i32>
-// CHECK: %[[VAL_7:.*]] = emitc.expression : i32 {
-// CHECK: %[[VAL_8:.*]] = load %[[VAL_6]] : <i32>
-// CHECK: yield %[[VAL_8]] : i32
+// CHECK: %[[VAL_7:.*]] = emitc.expression : i1 {
+// CHECK: %[[VAL_8:.*]] = load %[[VAL_3]] : <i32>
+// CHECK: %[[VAL_9:.*]] = add %[[VAL_8]], %[[VAL_5]] : (i32, i32) -> i32
+// CHECK: %[[VAL_10:.*]] = cmp lt, %[[VAL_9]], %[[VAL_0]] : (i32, i32) -> i1
+// CHECK: yield %[[VAL_10]] : i1
// CHECK: }
-// CHECK: %[[VAL_9:.*]] = emitc.expression : i1 {
-// CHECK: %[[VAL_10:.*]] = add %[[VAL_4]], %[[VAL_7]] : (i32, i32) -> i32
-// CHECK: %[[VAL_11:.*]] = cmp lt, %[[VAL_10]], %[[VAL_0]] : (i32, i32) -> i1
-// CHECK: yield %[[VAL_11]] : i1
-// CHECK: }
-// CHECK: return %[[VAL_9]] : i1
+// CHECK: return %[[VAL_7]] : i1
// CHECK: }
diff --git a/mlir/test/Target/Cpp/expressions.mlir b/mlir/test/Target/Cpp/expressions.mlir
index 9316d7b77619b..b07c5f1804d38 100644
--- a/mlir/test/Target/Cpp/expressions.mlir
+++ b/mlir/test/Target/Cpp/expressions.mlir
@@ -343,13 +343,13 @@ func.func @expression_with_subscript_user(%arg0: !emitc.ptr<!emitc.opaque<"void"
return %res_load : i32
}
-// CPP-DEFAULT: bool expression_with_load(int32_t [[VAL_1:v.+]], int32_t [[VAL_2:v.+]], int32_t* [[VAL_3:v.+]]) {
+// CPP-DEFAULT: bool expression_with_var_load_and_subscript(int32_t [[VAL_1:v.+]], int32_t [[VAL_2:v.+]], int32_t* [[VAL_3:v.+]]) {
// CPP-DEFAULT-NEXT: int64_t [[VAL_4:v.+]] = 0;
// CPP-DEFAULT-NEXT: int32_t [[VAL_5:v.+]] = 42;
// CPP-DEFAULT-NEXT: bool [[VAL_6:v.+]] = [[VAL_5]] + [[VAL_2]] < [[VAL_3]][[[VAL_4]]] + [[VAL_1]];
// CPP-DEFAULT-NEXT: return [[VAL_6]];
-// CPP-DECLTOP: bool expression_with_load(int32_t [[VAL_1:v.+]], int32_t [[VAL_2:v.+]], int32_t* [[VAL_3:v.+]]) {
+// CPP-DECLTOP: bool expression_with_var_load_and_subscript(int32_t [[VAL_1:v.+]], int32_t [[VAL_2:v.+]], int32_t* [[VAL_3:v.+]]) {
// CPP-DECLTOP-NEXT: int64_t [[VAL_4:v.+]];
// CPP-DECLTOP-NEXT: int32_t [[VAL_5:v.+]];
// CPP-DECLTOP-NEXT: bool [[VAL_6:v.+]];
@@ -358,7 +358,7 @@ func.func @expression_with_subscript_user(%arg0: !emitc.ptr<!emitc.opaque<"void"
// CPP-DECLTOP-NEXT: [[VAL_6]] = [[VAL_5]] + [[VAL_2]] < [[VAL_3]][[[VAL_4]]] + [[VAL_1]];
// CPP-DECLTOP-NEXT: return [[VAL_6]];
-func.func @expression_with_load(%arg0: i32, %arg1: i32, %arg2: !emitc.ptr<i32>) -> i1 {
+func.func @expression_with_var_load_and_subscript(%arg0: i32, %arg1: i32, %arg2: !emitc.ptr<i32>) -> i1 {
%c0 = "emitc.constant"() {value = 0 : i64} : () -> i64
%0 = "emitc.variable"() <{value = #emitc.opaque<"42">}> : () -> !emitc.lvalue<i32>
%ptr = emitc.subscript %arg2[%c0] : (!emitc.ptr<i32>, i64) -> !emitc.lvalue<i32>
@@ -373,6 +373,26 @@ func.func @expression_with_load(%arg0: i32, %arg1: i32, %arg2: !emitc.ptr<i32>)
return %result : i1
}
+// CPP-DEFAULT: bool expression_with_var_load(int32_t [[VAL_1:v.+]], int32_t [[VAL_2:v.+]]) {
+// CPP-DEFAULT-NEXT: int32_t [[VAL_3:v.+]] = 42;
+// CPP-DEFAULT-NEXT: return [[VAL_3]] + [[VAL_1]] < [[VAL_2]];
+
+// CPP-DECLTOP: bool expression_with_var_load(int32_t [[VAL_1:v.+]], int32_t [[VAL_2:v.+]]) {
+// CPP-DECLTOP-NEXT: int32_t [[VAL_3:v.+]];
+// CPP-DECLTOP-NEXT: [[VAL_3]] = 42;
+// CPP-DECLTOP-NEXT: return [[VAL_3]] + [[VAL_1]] < [[VAL_2]];
+
+func.func @expression_with_var_load(%arg0: i32, %arg1: i32) -> i1 {
+ %0 = "emitc.variable"() <{value = #emitc.opaque<"42">}> : () -> !emitc.lvalue<i32>
+ %result = emitc.expression : i1 {
+ %a = emitc.load %0 : !emitc.lvalue<i32>
+ %b = emitc.add %a, %arg0 : (i32, i32) -> i32
+ %d = emitc.cmp lt, %b, %arg1 :(i32, i32) -> i1
+ yield %d : i1
+ }
+ return %result : i1
+}
+
// CPP-DEFAULT: bool expression_with_load_and_call(int32_t* [[VAL_1:v.+]]) {
// CPP-DEFAULT-NEXT: int64_t [[VAL_2:v.+]] = 0;
// CPP-DEFAULT-NEXT: bool [[VAL_3:v.+]] = [[VAL_1]][[[VAL_2]]] + bar([[VAL_1]][[[VAL_2]]]) < [[VAL_1]][[[VAL_2]]];
|
|
@llvm/pr-subscribers-mlir Author: Kirill Chibisov (kchibisov) ChangesThe load side effect semantic depends on the operation that was used to declare the load's operand. In case of -- Not sure whether the logic of #144990 should be applied here, since the Full diff: https://github.com/llvm/llvm-project/pull/145011.diff 4 Files Affected:
diff --git a/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td b/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td
index 9ecdb74f4d82e..d88407a4d6726 100644
--- a/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td
+++ b/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td
@@ -966,6 +966,10 @@ def EmitC_LoadOp : EmitC_Op<"load", [CExpressionInterface,
let results = (outs AnyType:$result);
let assemblyFormat = "$operand attr-dict `:` type($operand)";
+
+ let extraClassDeclaration = [{
+ bool hasSideEffects();
+ }];
}
def EmitC_MulOp : EmitC_BinaryOp<"mul", []> {
diff --git a/mlir/lib/Dialect/EmitC/IR/EmitC.cpp b/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
index e602210c2dc6c..10e0d931e17fd 100644
--- a/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
+++ b/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
@@ -898,6 +898,20 @@ LogicalResult emitc::VariableOp::verify() {
return verifyInitializationAttribute(getOperation(), getValueAttr());
}
+//===----------------------------------------------------------------------===//
+// VariableOp
+//===----------------------------------------------------------------------===//
+
+bool emitc::LoadOp::hasSideEffects() {
+ auto *defOp = this->getOperand().getDefiningOp();
+ // The load side effect semantic depends on the operation that was used to
+ // declare the load's operand, `VariableOp` and `MemberOp` perform loading
+ // from the stack, thus we may consider them as not having side effect to
+ // make end code more readable by letting those loads to get inlined.
+ return !defOp ||
+ !(llvm::isa<VariableOp>(defOp) || llvm::isa<MemberOp>(defOp));
+}
+
//===----------------------------------------------------------------------===//
// YieldOp
//===----------------------------------------------------------------------===//
diff --git a/mlir/test/Dialect/EmitC/transforms.mlir b/mlir/test/Dialect/EmitC/transforms.mlir
index a38f396dad953..687ebe2cd3612 100644
--- a/mlir/test/Dialect/EmitC/transforms.mlir
+++ b/mlir/test/Dialect/EmitC/transforms.mlir
@@ -135,21 +135,18 @@ func.func @single_result_requirement() -> (i32, i32) {
// CHECK-SAME: %[[VAL_1:.*]]: !emitc.ptr<i32>) -> i1 {
// CHECK: %[[VAL_2:.*]] = "emitc.constant"() <{value = 0 : i64}> : () -> i64
// CHECK: %[[VAL_3:.*]] = "emitc.variable"() <{value = #emitc.opaque<"42">}> : () -> !emitc.lvalue<i32>
-// CHECK: %[[VAL_4:.*]] = emitc.expression : i32 {
-// CHECK: %[[VAL_5:.*]] = load %[[VAL_3]] : <i32>
-// CHECK: yield %[[VAL_5]] : i32
+// CHECK: %[[VAL_4:.*]] = emitc.subscript %[[VAL_1]]{{\[}}%[[VAL_2]]] : (!emitc.ptr<i32>, i64) -> !emitc.lvalue<i32>
+// CHECK: %[[VAL_5:.*]] = emitc.expression : i32 {
+// CHECK: %[[VAL_6:.*]] = load %[[VAL_4]] : <i32>
+// CHECK: yield %[[VAL_6]] : i32
// CHECK: }
-// CHECK: %[[VAL_6:.*]] = emitc.subscript %[[VAL_1]]{{\[}}%[[VAL_2]]] : (!emitc.ptr<i32>, i64) -> !emitc.lvalue<i32>
-// CHECK: %[[VAL_7:.*]] = emitc.expression : i32 {
-// CHECK: %[[VAL_8:.*]] = load %[[VAL_6]] : <i32>
-// CHECK: yield %[[VAL_8]] : i32
+// CHECK: %[[VAL_7:.*]] = emitc.expression : i1 {
+// CHECK: %[[VAL_8:.*]] = load %[[VAL_3]] : <i32>
+// CHECK: %[[VAL_9:.*]] = add %[[VAL_8]], %[[VAL_5]] : (i32, i32) -> i32
+// CHECK: %[[VAL_10:.*]] = cmp lt, %[[VAL_9]], %[[VAL_0]] : (i32, i32) -> i1
+// CHECK: yield %[[VAL_10]] : i1
// CHECK: }
-// CHECK: %[[VAL_9:.*]] = emitc.expression : i1 {
-// CHECK: %[[VAL_10:.*]] = add %[[VAL_4]], %[[VAL_7]] : (i32, i32) -> i32
-// CHECK: %[[VAL_11:.*]] = cmp lt, %[[VAL_10]], %[[VAL_0]] : (i32, i32) -> i1
-// CHECK: yield %[[VAL_11]] : i1
-// CHECK: }
-// CHECK: return %[[VAL_9]] : i1
+// CHECK: return %[[VAL_7]] : i1
// CHECK: }
diff --git a/mlir/test/Target/Cpp/expressions.mlir b/mlir/test/Target/Cpp/expressions.mlir
index 9316d7b77619b..b07c5f1804d38 100644
--- a/mlir/test/Target/Cpp/expressions.mlir
+++ b/mlir/test/Target/Cpp/expressions.mlir
@@ -343,13 +343,13 @@ func.func @expression_with_subscript_user(%arg0: !emitc.ptr<!emitc.opaque<"void"
return %res_load : i32
}
-// CPP-DEFAULT: bool expression_with_load(int32_t [[VAL_1:v.+]], int32_t [[VAL_2:v.+]], int32_t* [[VAL_3:v.+]]) {
+// CPP-DEFAULT: bool expression_with_var_load_and_subscript(int32_t [[VAL_1:v.+]], int32_t [[VAL_2:v.+]], int32_t* [[VAL_3:v.+]]) {
// CPP-DEFAULT-NEXT: int64_t [[VAL_4:v.+]] = 0;
// CPP-DEFAULT-NEXT: int32_t [[VAL_5:v.+]] = 42;
// CPP-DEFAULT-NEXT: bool [[VAL_6:v.+]] = [[VAL_5]] + [[VAL_2]] < [[VAL_3]][[[VAL_4]]] + [[VAL_1]];
// CPP-DEFAULT-NEXT: return [[VAL_6]];
-// CPP-DECLTOP: bool expression_with_load(int32_t [[VAL_1:v.+]], int32_t [[VAL_2:v.+]], int32_t* [[VAL_3:v.+]]) {
+// CPP-DECLTOP: bool expression_with_var_load_and_subscript(int32_t [[VAL_1:v.+]], int32_t [[VAL_2:v.+]], int32_t* [[VAL_3:v.+]]) {
// CPP-DECLTOP-NEXT: int64_t [[VAL_4:v.+]];
// CPP-DECLTOP-NEXT: int32_t [[VAL_5:v.+]];
// CPP-DECLTOP-NEXT: bool [[VAL_6:v.+]];
@@ -358,7 +358,7 @@ func.func @expression_with_subscript_user(%arg0: !emitc.ptr<!emitc.opaque<"void"
// CPP-DECLTOP-NEXT: [[VAL_6]] = [[VAL_5]] + [[VAL_2]] < [[VAL_3]][[[VAL_4]]] + [[VAL_1]];
// CPP-DECLTOP-NEXT: return [[VAL_6]];
-func.func @expression_with_load(%arg0: i32, %arg1: i32, %arg2: !emitc.ptr<i32>) -> i1 {
+func.func @expression_with_var_load_and_subscript(%arg0: i32, %arg1: i32, %arg2: !emitc.ptr<i32>) -> i1 {
%c0 = "emitc.constant"() {value = 0 : i64} : () -> i64
%0 = "emitc.variable"() <{value = #emitc.opaque<"42">}> : () -> !emitc.lvalue<i32>
%ptr = emitc.subscript %arg2[%c0] : (!emitc.ptr<i32>, i64) -> !emitc.lvalue<i32>
@@ -373,6 +373,26 @@ func.func @expression_with_load(%arg0: i32, %arg1: i32, %arg2: !emitc.ptr<i32>)
return %result : i1
}
+// CPP-DEFAULT: bool expression_with_var_load(int32_t [[VAL_1:v.+]], int32_t [[VAL_2:v.+]]) {
+// CPP-DEFAULT-NEXT: int32_t [[VAL_3:v.+]] = 42;
+// CPP-DEFAULT-NEXT: return [[VAL_3]] + [[VAL_1]] < [[VAL_2]];
+
+// CPP-DECLTOP: bool expression_with_var_load(int32_t [[VAL_1:v.+]], int32_t [[VAL_2:v.+]]) {
+// CPP-DECLTOP-NEXT: int32_t [[VAL_3:v.+]];
+// CPP-DECLTOP-NEXT: [[VAL_3]] = 42;
+// CPP-DECLTOP-NEXT: return [[VAL_3]] + [[VAL_1]] < [[VAL_2]];
+
+func.func @expression_with_var_load(%arg0: i32, %arg1: i32) -> i1 {
+ %0 = "emitc.variable"() <{value = #emitc.opaque<"42">}> : () -> !emitc.lvalue<i32>
+ %result = emitc.expression : i1 {
+ %a = emitc.load %0 : !emitc.lvalue<i32>
+ %b = emitc.add %a, %arg0 : (i32, i32) -> i32
+ %d = emitc.cmp lt, %b, %arg1 :(i32, i32) -> i1
+ yield %d : i1
+ }
+ return %result : i1
+}
+
// CPP-DEFAULT: bool expression_with_load_and_call(int32_t* [[VAL_1:v.+]]) {
// CPP-DEFAULT-NEXT: int64_t [[VAL_2:v.+]] = 0;
// CPP-DEFAULT-NEXT: bool [[VAL_3:v.+]] = [[VAL_1]][[[VAL_2]]] + bar([[VAL_1]][[[VAL_2]]]) < [[VAL_1]][[[VAL_2]]];
|
The load side effect semantic depends on the operation that was used to declare the load's operand. In case of `VariableOp` and `MemberOp` the operation storage is on the stack, thus can be considered to not have side effects compared to e.g. subscript operation.
kchibisov
force-pushed
the
emitc-relax-variable-load
branch
from
94d3902 to
d4ac0b6
Compare
|
+1 on the motivation for this patch, which is IIUC to enable more expression merging.
int f(int k) {
int a, b;
a = g(k);
b = h(a);
a = i(k);
return a + b;
}it would be illegal to move the first load from |
Wouldn't it break because of the Though, I can remove this rule.
Just to ensure, are you talking about moving of the |
Yes, my example is indeed a bit problematic as the
Actually I do mean that in the CExpression/ |
But in that case, it doesn't matter, since |
Not sure I follow: Semantically, the load op reads from memory regardless of memory location. Could you explain over the example above? |
|
What I say that moving So with your original example, it was hard to grasp the issue, I don't see how you can actually break things, since you just move and refer memory location, and you can not speculate with function calls. But if what you do is rely on the It also means that the current issue is present with the So the problem is that generally the scheduler is not aware of the context over which it tries to move things, and generally should account for memory writes between the origin and destination of the moved instruction due to inlining. So for inliner to work correctly, we should have To sum up, unless the inliner accounts for writes, 1.) said patch can not go 2.) I believe that |
I don't quite get why UB matter for side-effects? |
|
@joker-eph I'm not sure you were around previous discussion when
The way
I said the exact same thing in the latest comment and pointed out that present operations also don't really work good with the current design, and it needs restriction and maybe a pass to relax that. The general pipeline is that By iniling I mean, that the |
Can you link to the previous discussion?
The suspicious part is to make a load not having side-effects so that you get the effect you want on the transformation, while it does not seems intrinsically tied to the semantics property of the load itself (or at least I didn't read a description justifiying it clearly in the patch: saying "because local variable" seems a bit short on detail to be convincing).
I got this part, but I didn't get how/why loading from a local variable is any different from other memory location to decide this, do you have motivating examples? |
The discussion started from there #91475 (comment) And I've also send #130802 , so you can manually use The end goal, at least the one I care about, is to make code more human looking, since I use this code to actually generate C as the end target, which is later used for static analysis purposes, so looking something close-ish to how you can write it is welcomed.
because you can not guarantee the order of execution within expression, and the goal was to make those predictable, but local variables have non of those issues, hence why trying to allow it, the rest is complete no-go for upstream. The end goal with this to make code more readable by inlining redundant binds when possible while preserving the we can not have expression which evaluation is not clearly defined. I don't really like the way it works right now in general when it comes to inline, etc, and as I stated above, the |
|
The problem is more like 1) And they both rely on |
Thanks, this is somehow what I expected, other than the part about local variables.
Why do local variables have non of those issues? I don't get this part actually. |
// Ok
int v1 = 5;
int v2 = v1;
int v3 = 10;
int v4 = v3;
// It's ok to just use `v1` and `v3`. Order doesn't matter, since `variable` is `copy` thing.
return v2 + v4;
// Not ok
int *v1 = (int *)malloc(10);
int v2 = v1[0];
int v3 = v1[1];
// It's not ok to use v1[0] and v1[1], since order is not defined.
return v2 + v3;This is also present in the link and link from there. |
|
Thanks for the example! Can you explain why is the second example not OK? |
|
I think the more clear one is with function, actually. int *v1 = (int *)malloc(10);
int v2 = v1[0];
int v3 = my_func_with_effect(10);
// It's not ok to use v1[0] and my_func_with_effect(10), since order is not defined.
return v2 + v3;So the problem is that it's not clear, what exactly will be evaluated first, v1[0] or I personally don't have issues with any of that (and you can write by hand cexpressions like that as well), it's just the problem when you translate code that was strictly structured before into cexpressions, thus not really preserving the order in cases like that. So, the whole problem is doing it automatically, since you had strict order before the transformations and according to the fact that it's not defined, it matters in such cases, where you have e.g. func calls, array accesses along side the loads, etc, etc). |
|
The problem is that this comment talks about "sequencing multiple loads", which unless they are volatile, shouldn't be a problem (I'm willing to see a counter example...). The real problem I believe is sequencing reads and writes.
I still have the same question as before: please describe the problem more explicitly here:
I will also jump a bit a head of the next few back-and-forth we'll have on this topic and ask you to consider: Consider why inlining is not OK despite the use a local variable. |
|
Well, they clearly talk about the same thing I'm talking, if you look at the example and the proposed change #91475 (comment) To generate e.g. int f(int a[300], int b[700], int i, int j) {
int8_t v1;
int8_t v2;
int8_t v2;
// v1 = a[i];
// v2 = b[j];
// return v1 * v2;
return (v1 = a[i], v2 = b[j], v1 * v2);
}so the
The problem is exactly https://en.cppreference.com/w/c/language/eval_order.html . Before form-expression and iniling the order was I don't have other explanation, other than what I said before that by inlining/forming cexpression we change the order of evaluation of the program. |
The load side effect semantic depends on the operation that was used to declare the load's operand. In case of
VariableOpandMemberOpthe operation storage is on the stack, thus can be considered to not have side effects compared to e.g. subscript operation.--
Not sure whether the logic of #144990 should be applied here, since the
variable/memberops are rather limited iirc. cc @jacquesguan.