[Mlir-commits] [mlir] [mlir] Add support for DIGlobalVariable and DIGlobalVariableExpression (PR #73367)

[Tobias Gysi]

================
@@ -109,3 +119,251 @@ bool MemoryEffectsAttr::isReadWrite() {
     return false;
   return true;
 }
+
+//===----------------------------------------------------------------------===//
+// DIExpression
+//===----------------------------------------------------------------------===//
+
+DIExpressionAttr DIExpressionAttr::get(MLIRContext *context) {
+  return get(context, ArrayRef<uint64_t>({}));
+}
+
+#define _FORMAT_DW_OP(x) #x
+#define FORMAT_DW_OP(NAME, VENDOR) _FORMAT_DW_OP(DW_OP_##VENDOR##_##NAME)
+
+size_t queryNumOpParams(llvm::dwarf::LocationAtom op) {
+  switch (op) {
+  case llvm::dwarf::DW_OP_addr:
+  case llvm::dwarf::DW_OP_const1u:
+  case llvm::dwarf::DW_OP_const1s:
+  case llvm::dwarf::DW_OP_const2u:
+  case llvm::dwarf::DW_OP_const2s:
+  case llvm::dwarf::DW_OP_const4u:
+  case llvm::dwarf::DW_OP_const4s:
+  case llvm::dwarf::DW_OP_const8u:
+  case llvm::dwarf::DW_OP_const8s:
+  case llvm::dwarf::DW_OP_constu:
+  case llvm::dwarf::DW_OP_consts:
+  case llvm::dwarf::DW_OP_pick:
+  case llvm::dwarf::DW_OP_plus_uconst:
+  case llvm::dwarf::DW_OP_bra:
+  case llvm::dwarf::DW_OP_skip:
+  case llvm::dwarf::DW_OP_breg0:
+  case llvm::dwarf::DW_OP_breg1:
+  case llvm::dwarf::DW_OP_breg2:
+  case llvm::dwarf::DW_OP_breg3:
+  case llvm::dwarf::DW_OP_breg4:
+  case llvm::dwarf::DW_OP_breg5:
+  case llvm::dwarf::DW_OP_breg6:
+  case llvm::dwarf::DW_OP_breg7:
+  case llvm::dwarf::DW_OP_breg8:
+  case llvm::dwarf::DW_OP_breg9:
+  case llvm::dwarf::DW_OP_breg10:
+  case llvm::dwarf::DW_OP_breg11:
+  case llvm::dwarf::DW_OP_breg12:
+  case llvm::dwarf::DW_OP_breg13:
+  case llvm::dwarf::DW_OP_breg14:
+  case llvm::dwarf::DW_OP_breg15:
+  case llvm::dwarf::DW_OP_breg16:
+  case llvm::dwarf::DW_OP_breg17:
+  case llvm::dwarf::DW_OP_breg18:
+  case llvm::dwarf::DW_OP_breg19:
+  case llvm::dwarf::DW_OP_breg20:
+  case llvm::dwarf::DW_OP_breg21:
+  case llvm::dwarf::DW_OP_breg22:
+  case llvm::dwarf::DW_OP_breg23:
+  case llvm::dwarf::DW_OP_breg24:
+  case llvm::dwarf::DW_OP_breg25:
+  case llvm::dwarf::DW_OP_breg26:
+  case llvm::dwarf::DW_OP_breg27:
+  case llvm::dwarf::DW_OP_breg28:
+  case llvm::dwarf::DW_OP_breg29:
+  case llvm::dwarf::DW_OP_breg30:
+  case llvm::dwarf::DW_OP_breg31:
+  case llvm::dwarf::DW_OP_regx:
+  case llvm::dwarf::DW_OP_fbreg:
+  case llvm::dwarf::DW_OP_piece:
+  case llvm::dwarf::DW_OP_deref_size:
+  case llvm::dwarf::DW_OP_xderef_size:
+  case llvm::dwarf::DW_OP_call2:
+  case llvm::dwarf::DW_OP_call4:
+  case llvm::dwarf::DW_OP_call_ref:
+  case llvm::dwarf::DW_OP_addrx:
+  case llvm::dwarf::DW_OP_constx:
+  case llvm::dwarf::DW_OP_convert:
+  case llvm::dwarf::DW_OP_reinterpret:
+    return 1;
+  case llvm::dwarf::DW_OP_bregx:
+  case llvm::dwarf::DW_OP_bit_piece:
+  case llvm::dwarf::DW_OP_implicit_value:
+  case llvm::dwarf::DW_OP_implicit_pointer:
+  case llvm::dwarf::DW_OP_entry_value:
+  case llvm::dwarf::DW_OP_regval_type:
+  case llvm::dwarf::DW_OP_deref_type:
+  case llvm::dwarf::DW_OP_xderef_type:
+    return 2;
+  case llvm::dwarf::DW_OP_const_type:
+    return 3;
+  default:
+    return 0;
+  }
+}
+
+::mlir::Attribute DIExpressionAttr::parse(AsmParser &parser, Type type) {
----------------
gysit wrote:

``> I'm almost positive I won't be able to get this to work using the existing stuff implemented in tablegen. In order to do this properly, I would need to somehow store both the value and what DWARF kind that value is together. THEN in order to support printing the textual form, you'd need to look at the kind and choose the correct printer.

Yes there is no good support for such a sum type I think. The best that comes to my mind for storing one value in the DIExpression array could be to have an attribute that contains optional fields for the enums we support and an optional integer field that is used for values that do not map to any of the enums:
```
def LLVM_DIExpressionElemAttr : LLVM_Attr<"DIExpressionElem", "di_expression_elem"> {
  let parameters = (ins 
    OptionalParameter<LLVM_DIOpcodeParameter>:$opcode,
    OptionalParameter<LLVM_DIEncodingParameter>:$encoding,
    OptionalParameter<"int64_t">:$value
  );
}
```
Did not try this but in principle it should work. Also note that this would be a flat representation that does not group the integers that belong together. The import / export of this should be fairly easy since we can use the existing LLVM methods to check if an integer value maps to a specific enum.

Another approach could be to add a custom printer and parser to the existing DIExpression that during printing / parsing checks for every integer if there is a string representation (again using the LLVM functions that are used in the printers and parsers of LLVM_DIParameter. I think this is the lowest complexity solution which stays close to LLVM and the printer / parser should have acceptable complexity. It is clearly less typed than the "sum type" approach.

Is the grouping you implemented in queryNumOpParams necessary? Isn't there an LLVM function we can query to get the number of parameters of an opcode? Overall it seems like this adds quite a bit of complexity and maybe an easy approach could be to just use a flat list for the DIExpression?

 > I think for the sake of getting this out, I'd rather not spend more time on getting expressions 1-to-1 with LLVM's representation.

Also feel free to use DIExpression more or less as is yes. This can also be improved in a separate PR later on and you can use this PR to introduce the GlobalVariableExpression.

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