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

[Gil Rapaport]

aniragil wrote:

> Track side effects inside the emitter, since everything is already in place there and the cache is there. The load operation could check whether it has side effect or not and could be inlined if there's no side effect or user asked to inline anyway potentially resulting in _undefined_ evaluation in the generated code.

We're actually in the process of making the translator follow MLIR's guidelines, which are to be as simple and as 1-1 as possible, leaving all legalization/optimization work to passes.

> Define a trait `HasLoadSideEffect`, which could be used to unsure that load's operand inside the `CExpression` is not having this trait.

AFAIK traits shouldn't be dynamic, but that's a technicality as we could use attributes instead. However, that still leaves the translator to do this transformation, which is less desired.

> As a side note, I think _mine_ use case is quite different to what you're going for with lvalue, since we want something _more readable_ being generated in the end and looking like something a _regular person could write_, doing explicit bindings for subscript is certainly not that (I'd use `at` or something that does bounds checking). Though, having more _defined_ things generated is a _good default_ from what I can say.

Actually, making the emitted C code more readable and closer to how humans write it is very important to me as well (and the reason I added `emitc.expression`), good to know others interested in raising the bar in that aspect. Now that the lvalues change is in we could look into incorporating lvalues into expressions as well.

Also, could you elaborate on your motivation for using --declare-variables-at-top? Is it part of your readability preference or functionally required, e.g. by your target C compiler?

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