[Mlir-commits] [mlir] [mlir][emitc] Fix creating pointer from constant array (PR #162083)

[Gil Rapaport]

================
@@ -1,5 +1,11 @@
 // RUN: mlir-translate -mlir-to-cpp %s | FileCheck %s -check-prefix=CPP-DEFAULT
 // RUN: mlir-translate -mlir-to-cpp -declare-variables-at-top %s | FileCheck %s -check-prefix=CPP-DECLTOP
+// check that the generated code is syntactically valid
+// RUN: mlir-translate -mlir-to-cpp %s | %host_cxx -fsyntax-only -x c++ -
----------------
aniragil wrote:

So there are some aspects here I think might be worth a wider discussion:

- Implementation: IIUC the `%host_cxx` is the compiler (and version) used for building LLVM itself. It won't necessarily align with EmitC's restrictions, and we'd also be adding to it a new use-case unknown to folks not involved in EmitC.

- Compile time: going through a C++ frontend may affect the total runtime of `check-mlir`.

- Scope: As in this case, making the result pass the frontend requires adding code which isn't part of the test (in this case include directives), which goes against the LLVM guideline to keep LIT tests minimal.

- Are LIT tests the right tool for this? AFAIK LIT tests don't usually make sure the expected result is "semantically" correct - in this case that the emitted code is valid C code - only that it is as expected (however the SPIRV translator does seem to do that: its LIT tests run SPIRV validation under a `%if spirv-tools` condition). Alternatives to LIT tests might be LLVM's e2e test-suite or setting up a dedicated buildbot.

Would be good to hear what others think about this, e.g. on discourse (and if such a mechanism is constructed, introduce it in its own PR consistently for all relevant tests). I'd leave it out of this PR, both to get the bug fix in asap and to avoid having the bug fix reverted if this kind of testing proves problematic. WDYT?

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