LLVMOps.td: why only some operations have an llvmBuilder?

Hello,

I am working on adding alignment to the GlobalOp operation of the LLVM dialect of MLIR. Following critical, but very reasonable feedback from @ftynse , I try to fully mimick the treatment of alignment of AllocaOp. In doing this, I noticed that differences between GlobalOp and AllocaOp go beyond just alignment. For instance, GlobalOp does not seem to have an llvmBuilder which, funny enough, seems to be not an MLIR builder, but an LLVM builder.

I therefore have a few questions:

  • is there a doc on the various constructs of the tablegen files used in MLIR?
  • is the llvmBuilder used to allow the automatic generation of the LLVM operation from its MLIR counterpart?
  • why some operations, such as GlobalOp, do not have an llvmBuilder?

Best,
Dumitru

I’m afraid there’s no dedicated documentation for that other than inline documentation in the definition of LLVM_Op. There isn’t much difference between LLVM dialect ops and other ODS definitions otherwise.

As you correctly noticed, llvmBuilder is there to construct LLVM IR not MLIR hence the name. The definitions are processed by llvm-project/LLVMIRConversionGen.cpp at main · llvm/llvm-project · GitHub to generate a long if-else spaghetti that is included into llvm-project/LLVMToLLVMIRTranslation.cpp at 761f3d16753ecea625173729dd8c53df022cd4ab · llvm/llvm-project · GitHub. The goal of the exercise, as usual with Tablegen, is to avoid repeatedly writing too much boilerplate C++.

Some operations don’t have the llvmBuilder field set because they are handled directly in the C++ code in which the contents of that field would be included. Typically, the C++ handling code is longer than a couple of lines and is therefore discouraged to be placed inside a string in a .td file. We are doing the same for builder bodies, interfaces, etc.

Furthermore, GlobalOp is special (similarly to FuncOp) because it is processed differently in the translation https://github.com/llvm/llvm-project/blob/main/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp#L409 – it is a named symbol that needs to be looked up when translating AddressOfOp (so all global names must be defined before translating any “normal” operation), it’s placed in the module, etc.

So, if I understand well, it’s normal that GlobalOp does not define an llvmBuilder. But then, I have a “standardization” problem, because it would seem natural for GlobalOp to derive from MemoryOpWithAlignmentBase (which seems to be meant just for ops like GlobalOp). However, the only ops using it are AllocaOp, LoadOp, and StoreOp, in the llvmBuilder code. It’s kind of puzzling. Can you explain me how to advance on this point?

Don’t make GlobalOp derive from MemoryOpWithAlignmentBase. The latter is there, again, to avoid copy-pasting now boilerplate Tablegen in multiple ops. Even the Tablegen backend doesn’t care if that class is present or not. Ultimately, all of that produces C++ code so one may just as well write C++ directly. If you think that the code stanza in MemoryOpWithAlignmentBase can be reused for GlobalOp, move it to a static function inside a .cpp and change the class to call that function + call it from GlobalOp translation.

@ftynse I have another problem: it seems that someone (you?) had already added an llvm.align attribute to the LLVMIR dialect of MLIR, which is only used in some tests where it is directly converted to the align attribute of LLVM IR. Should I simply ignore it exists? Also note that it has (as you seemed to want at some point) a getAlignAttrName method fixing its name. I could also delete it altogether or replace its instances with alignment. Or convert my attribute to llvm.align.

Not me, ⚙ D82161 Add support for alignment annotations to the LLVM dialect to LLVM translation., but I reviewed the patch.

Yes, you should ignore this attribute. This is a discardable attribute that can be attached to function arguments. What you are adding is an inherent attribute with a similar purpose. See the difference here MLIR Language Reference - MLIR. These two kinds of attributes are different semantically and even have different names.

Ok. Just a question: what happens if you have both “alignment=32” and “llvm.align=64” on an operation such as an “alloca”? The semantics may be different, but it conflicts.

Since llvm.align is a discardable attribute, it can be discarded at any time as its name indicates. Translation ignores it outside of function arguments, for example.

We can consider restricting its usage to function-like ops, but this isn’t really in scope of the change you’d like to do so you don’t have to bother about it.

Ok, I have finished my revision. My suggestion for the future would be to remove one of alignment and llvm.align. This would allow the introduction of a standard attribute name (like for “linkage”).