[Mlir-commits] [mlir] [mlir][EmitC] Model lvalues as a type in EmitC (PR #91475)

[Gil Rapaport]

aniragil wrote:

So I think there are two issues here:

First, we currently don't fold expressions with side effects into their using expressions at all, since we might be moving side effects beyond other, potentially conflicting side effects, e.g.:

```C
int f(int a, int b) {
  int8_t v1;      // actual variable
  int8_t v2;      // "SSA-value" variable
  int8_t *v3;     // "SSA-value" variable
  v1 = a;         // emitc.assign
  v3 = &v1;       // emitc.apply "&"
  v2 = v1;        // emic.load
  func(v3);       // may modify v1
  v4 = v2 * 3;    // emitc.expression
  return v4;
}
```

(Note however that we do fold *into* expressions with side effects)
We're being highly conservative, and can definitely try to relax that for cases we can prove semantics is retained, including for variable loading.

Second, folding more than a single side-effect into an expression brings up the evaluation order issue @simon-camp mentioned.
As lvalues are not just variables, a load op used for an array access may lead to a invalid memory access and trigger an error/exception. I'm not sure compilers must respect evaluation order in illegal access cases, but AFAIK for variables (automatic / global) there is no such concern so I believe @mgehre-amd's observation is right for variables, at least for C and C++'s native types (not sure this holds for C++ classes whose copy constructors might do all sorts of stuff internally).
In general, we could use the comma operator to enforce evaluation order within expressions. While I don't think we can define variables inside comma expressions, specifically when --declare-variables-at-top is used we could fold dangerous loads in-order using commas, e.g.

```C
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);
}
```

Not sure it looks better than the alternative though.


https://github.com/llvm/llvm-project/pull/91475