search

llvm / clangir

A new (MLIR based) high-level IR for clang.
https://clangir.org
Other
386 stars 98 forks source link

Implement MLIR visibility in terms of CIR visibility #1029

Open bcardosolopes opened 3 weeks ago

bcardosolopes commented 3 weeks ago

MLIR one handles public, private and nested, CIR instead has all we need for C/C++ semantics + LLVM lowering.

The initial plan when we introduced CIR visibility was to later remove explicit reference to MLIR one, and implement the necessary interfaces in terms of CIR instead (also because we don't want to print/parse both on globals/functions, sounds like silly and unnecessary bloating). @roro47 did the first part, which is to introduce CIR visibility, but she's not working on CIR anymore.

The missing steps from previous plan of record is (hopefully still makes sense):

  1. Add a CIR visibility attribute, print and parse and fix tests for FuncOp and GlobalOp (ignore MLIR visibility for now).
  2. Migrate CIRGen properties that are currently querying MLIR visibility, to instead query the attribute representing CIR visibility (like some linkage stuff currently does). Perhaps here you introduce extraClassDeclarations to this new attribute or add some type of CIR interface (one example is the AST interface or GlobalValue).
  3. Remove printing and parser for MLIR visibility while providing custom implementation of the interface methods here: mlir/include/mlir/IR/SymbolInterfaces.td. We need to keep sym name and visibility but the requirements are not as many: https://mlir.llvm.org/docs/SymbolsAndSymbolTables/#defining-or-declaring-a-symbol
  4. At this point MLIR visibility is only important when using the symbol interface to query information. Anything query done on top of CIR should come through it's attributes.

cc @smeenai @seven-mile

bcardosolopes commented 3 weeks ago

Fixing this should take into account the extra whitespaces for cir.global, @seven-mile already fixed cir.func in https://github.com/llvm/clangir/pull/1028