[CIR][Interface] introduce CIRGlobalValueInterface for GlobalOp and FuncOp

[ghehg]

CIRGlobalValueInterface inherits from mlir::Symbol as it should, and GlobalOp and FuncOp now has interface mlir::Symbol through CIRGlobalValueInterface

and this PR basically make function isDeclarationForLinker into the CIRGlobalValueInterface interface. We also change some call sites of isDeclaration to use CIRGlobalValueInterface when its appropriate.

[bcardosolopes]

Thanks for improving this @ghehg, way to go. Some minor nits to address in inline comments.

While you are here, more changes you could do in followup PRs:

• GetAddrOfGlobal should return the global value interface.
• mlir::cir::isAvailableExternallyLinkage and friends from CIROpsEnums.h should be part of the interface instead of free functions (those were a hack because we didn't have an interface before).

[bcardosolopes]

Nice formatting, thanks. Note that changes to CIRCallOpInterface are unrelated to the patch and makes it harder to review. Fine for this PR because you didn't know but please avoid doing that in the future, better do formatting/refactoring on their own PRs.

[bcardosolopes]

Given both hasAvailableExternallyLinkage impls are equal, just make this the default impl in the td file, and delete the one from GlobalOp.

[ghehg]

I actually tried that. But it fails to compile as tbgen doesn't recognize getLinkage(), and getLinkage() seems to be a getter for OP attribute "linkage". Would love to learn how we deal with this situation as I totally agree with you this code dup is ugly.

[bcardosolopes]

Maybe something along the lines of what we see in CIR_CallOp could be doable:

    /// Return the callee of this operation
    CallInterfaceCallable getCallableForCallee() {
      return (*this)->getAttrOfType<SymbolRefAttr>("callee");
    }

Note that because follow up work will make mlir::cir::isAvailableExternallyLinkage be part of the interface as well, perhaps you can address this as part of that change instead.

[bcardosolopes]

You can delete the TODO as well. I think this whole change could be further simplified by:

auto cirGlobalValue = cast<mlir::cir::CIRGlobalValueInterface>(globalValueOp);
if (!cirGlobalValue.isDeclaration())
  return;

See the original code in CodeGenModule::EmitDeferred().

[ghehg]

Just to see if I understand it correctly. Does this mean "dyn_castmlir::cir::GetGlobalOp(Op)" should not be a valid check here? I.E. early logic in the function should make sure Op is of CIRGlobalValueInterface (either GlobalOp or FuncOp)
Actually, I had that doubt when I changed this code,
now as you suggested, after comparing with code in lib/clang/CodeGen/CodeGenModule.cpp
such as
void CodeGenModule::EmitDeferred() {
and CodeGenModule::GetAddrOfGlobalVar

I kinda question why CIR counterpart CIRGenModule::getAddrOfGlobalVar is returning GetGlobalOp insteadof GlobalOp?

[bcardosolopes]

My bad, I read GetGlobalOp as GlobalOp, I take back what I said :)

I kinda question why CIR counterpart CIRGenModule::getAddrOfGlobalVar is returning GetGlobalOp insteadof GlobalOp?

The OG one returns llvm::Constant *, which if we think in CIR terms, there isn't a good direct counterpart - it could be either and attribute (globalview) or an actual mlir::value result of getglobalop, so not really translatable at the time we wrote this.

If you have better ideas on how to go about this, it's definitely an improvement we could do in its own PR.

[bcardosolopes]

My bad, I read GetGlobalOp as GlobalOp, I take back what I said :)

I kinda question why CIR counterpart CIRGenModule::getAddrOfGlobalVar is returning GetGlobalOp insteadof GlobalOp?

The OG one returns llvm::Constant *, which if we think in CIR terms, there isn't a good direct counterpart - it could be either and attribute (globalview) or an actual mlir::value result of getglobalop, so not really translatable at the time we wrote this.

If you have better ideas on how to go about this, it's definitely an improvement we could do in its own PR.

[bcardosolopes]

Maybe something along the lines of what we see in CIR_CallOp could be doable:

    /// Return the callee of this operation
    CallInterfaceCallable getCallableForCallee() {
      return (*this)->getAttrOfType<SymbolRefAttr>("callee");
    }

Note that because follow up work will make mlir::cir::isAvailableExternallyLinkage be part of the interface as well, perhaps you can address this as part of that change instead.