MLIR: Add Support to compile flags output MLIR file

[Robertorosmaninho]

No description provided.

[thewilsonator]

You will have to put all of your code behind a #ifdef for versions of LDC that are not built with MLIR enabled (i.e. old verions of LLVM and those built without MLIR support).

[JohanEngelen]

You will have to put all of your code behind a #ifdef for versions of LDC that are not built with MLIR enabled (i.e. old verions of LLVM and those built without MLIR support).

You can restrict #ifdef to the stuff that actually requires MLIR library calls. The rest of the code (like the commandline parsing) you can keep, and then let LDC output an error message saying that MLIR is not supported in case LDC was built without it (for which you do need to #ifdef stuff ;-) ).

[JohanEngelen]

why is this code not part of writeModule? (see e.g. line 415).

[Robertorosmaninho]

Just to have it in different functions, but I can also put it on writeModule, the problem is that I'll need some extra arguments on this function, so the function should be something like:

void writeModule(llvm::Module *m, const char *filename, ::Module *module,
    mlir::MLIRContext &mlirContext, IRState *irs);

In addition, writeAndFreeLLModulefrom CodeGenerator must also be changed to receive a dmodule as an argument to be used on write module

[JohanEngelen]

@Robertorosmaninho I looked more into this. I think I was confused and actually it looks like you have to fully rewrite the IR building code. I thought the same code can output IR and MLIR, but now it appears to me that those are completely separate IR build libraries. So you'd have to rewrite all D AST->MLIR generation from scratch. In that case, there is no point in trying to merge things into toobj.cpp. You have to create a new codegenerator, basically duplicate class CodeGenerator functionality. That is quite something different from DCompute codegen, and will require a lot of work. Start by creating a class MLIRCodeGenerator and have it output an mainly empty mlir file. Then slowly make it emit more and more stuff.

[JohanEngelen]

See my inline comments, otherwise looks good (I still don't like the addition to codegenerator, but let's live with it for now).
Main thing that is missing is test case(s).

[Robertorosmaninho]

See my inline comments, otherwise looks good (I still don't like the addition to codegenerator, but let's live with it for now).
Main thing that is missing is test case(s).

Could you point me some test cases that test whether "file.ll" was created?

[JohanEngelen]

Could you point me some test cases that test whether "file.ll" was created?

Create folder tests/mlir and add your testcase there. Only enable that folder when MLIR is enabled: See how things are done for another conditional built-in capability like dynamic compile: https://github.com/ldc-developers/ldc/blob/master/tests/lit.site.cfg.in#L61-L63

Add a testcase, similar to anything in tests/codegen, like https://github.com/ldc-developers/ldc/blob/master/tests/codegen/wasi.d
More about testcases: https://wiki.dlang.org/LDC_Lit-based_testsuite

[Robertorosmaninho]

Could you point me some test cases that test whether "file.ll" was created?

Create folder tests/mlir and add your testcase there. Only enable that folder when MLIR is enabled: See how things are done for another conditional built-in capability like dynamic compile: https://github.com/ldc-developers/ldc/blob/master/tests/lit.site.cfg.in#L61-L63

Thanks!

Add a testcase, similar to anything in tests/codegen, like https://github.com/ldc-developers/ldc/blob/master/tests/codegen/wasi.d
More about testcases: https://wiki.dlang.org/LDC_Lit-based_testsuite

These tests will be presented with the modifications that actually generate some code, another branch is already being prepared to be sent here as PR (https://github.com/Robertorosmaninho/ldc/tree/Introducing-MLIR-translation/tests/codegen_mlir).

[JohanEngelen]

Overall looks OK now after fixing my remarks. Looking forward to the next PR that adds actual output testing.

[JohanEngelen]

lgtm

Johan is satisied

[kinke]

(ldc2 size increase is < 1% for the Azure packages, so no need to postpone this after v1.22 final.)

Thanks Roberto for hanging on, and the (much needed :)) reviewers. Looking forward to the follow ups.