add cir parsing test, and fix more coding style issues

clang/lib/CIR/CodeGen/CIRGenModule.cpp

@@ -488,7 +488,7 @@ void CIRGenModule::buildGlobal(GlobalDecl GD) {
     if (FD->hasAttr<AnnotateAttr>()) {
       StringRef MangledName = getMangledName(GD);
       if (getGlobalValue(MangledName))
-        DeferredAnnotations[MangledName] = FD;
+        deferredAnnotations[MangledName] = FD;
     }
     // Forward declarations are emitted lazily on first use.
     if (!FD->doesThisDeclarationHaveABody()) {
@@ -602,7 +602,7 @@ void CIRGenModule::buildGlobalFunctionDefinition(GlobalDecl GD,
     AddGlobalDtor(Fn, DA->getPriority(), true);
 
   if (D->getAttr<AnnotateAttr>())
-    DeferredAnnotations[getMangledName(GD)] = cast<ValueDecl>(D);
+    deferredAnnotations[getMangledName(GD)] = cast<ValueDecl>(D);
 }
 
 /// Track functions to be called before main() runs.
@@ -3199,23 +3199,23 @@ mlir::ArrayAttr CIRGenModule::buildAnnotationArgs(const AnnotateAttr *attr) {
   for (Expr *e : exprs) {
     id.Add(cast<clang::ConstantExpr>(e)->getAPValueResult());
   }
-  mlir::ArrayAttr &lookup = AnnotationArgs[id.ComputeHash()];
+  mlir::ArrayAttr &lookup = annotationArgs[id.ComputeHash()];
   if (lookup)
     return lookup;
 
   llvm::SmallVector<mlir::Attribute, 4> args;
   args.reserve(exprs.size());
   for (Expr *e : exprs) {
-    if (const auto StrE =
+    if (const auto strE =
             ::clang::dyn_cast<clang::StringLiteral>(e->IgnoreParenCasts())) {
       // Add trailing null character as StringLiteral->getString() does not
-      args.push_back(builder.getStringAttr(StrE->getString()));
-    } else if (const auto IntE = ::clang::dyn_cast<clang::IntegerLiteral>(
+      args.push_back(builder.getStringAttr(strE->getString()));
+    } else if (const auto intE = ::clang::dyn_cast<clang::IntegerLiteral>(
                    e->IgnoreParenCasts())) {
       args.push_back(mlir::IntegerAttr::get(
           mlir::IntegerType::get(builder.getContext(),
-                                 IntE->getValue().getBitWidth()),
-          IntE->getValue()));
+                                 intE->getValue().getBitWidth()),
+          intE->getValue()));
     } else {
       llvm_unreachable("NYI");
     }
@@ -3228,7 +3228,7 @@ mlir::ArrayAttr CIRGenModule::buildAnnotationArgs(const AnnotateAttr *attr) {
 
 mlir::cir::AnnotationAttr
 CIRGenModule::buildAnnotateAttr(const AnnotateAttr *aa) {
-  auto annoGV = builder.getStringAttr(aa->getAnnotation());
+  mlir::StringAttr annoGV = builder.getStringAttr(aa->getAnnotation());
   mlir::ArrayAttr args = buildAnnotationArgs(aa);
   return mlir::cir::AnnotationAttr::get(builder.getContext(), annoGV, args);
 }
@@ -3248,10 +3248,10 @@ void CIRGenModule::addGlobalAnnotations(const ValueDecl *d,
 }
 
 void CIRGenModule::buildGlobalAnnotations() {
-  for (const auto &[MangledName, VD] : DeferredAnnotations) {
+  for (const auto &[MangledName, VD] : deferredAnnotations) {
     mlir::Operation *GV = getGlobalValue(MangledName);
     if (GV)
       addGlobalAnnotations(VD, GV);
   }
-  DeferredAnnotations.clear();
+  deferredAnnotations.clear();
 }

clang/lib/CIR/CodeGen/CIRGenModule.h

@@ -131,20 +131,17 @@ class CIRGenModule : public CIRGenTypeCache {
   /// Annotations
   /// -------
 
-  /// We do not store global annotations in the module as annotation is
-  /// represented as attribute of GlobalOp, we defer creation of global
-  /// annotation variable to LoweringPrepare as CIR passes do not
-  /// need to have a global view of all annotations.
-
-  /// Map used to get unique annotation related strings.
-  llvm::StringMap<mlir::StringAttr> AnnotationStrings;
+  /// We do not store global annotations in the module here, instead, we store
+  /// each annotation as attribute of GlobalOp and FuncOp.
+  /// We defer creation of global annotation variable to LoweringPrepare
+  //  as CIR passes do not need to have a global view of all annotations.
 
   /// Used for uniquing of annotation arguments.
-  llvm::DenseMap<unsigned, mlir::ArrayAttr> AnnotationArgs;
+  llvm::DenseMap<unsigned, mlir::ArrayAttr> annotationArgs;
 
   /// Store deferred function annotations so they can be emitted at the end with
   /// most up to date ValueDecl that will have all the inherited annotations.
-  llvm::DenseMap<StringRef, const ValueDecl *> DeferredAnnotations;
+  llvm::DenseMap<StringRef, const ValueDecl *> deferredAnnotations;
 
 public:
   mlir::ModuleOp getModule() const { return theModule; }
@@ -780,7 +777,7 @@ class CIRGenModule : public CIRGenTypeCache {
 
   /// Emit all the global annotations.
   /// This actually only emits annotations for deffered declarations of
-  /// functions, because global variables need to deffred emission.
+  /// functions, because global variables need no deffred emission.
   void buildGlobalAnnotations();
 
   /// Emit additional args of the annotation.
@@ -792,8 +789,8 @@ class CIRGenModule : public CIRGenTypeCache {
   /// one of them.
   mlir::cir::AnnotationAttr buildAnnotateAttr(const AnnotateAttr *aa);
 
-  /// Add global annotations that are set on D, for the global GV. Those
-  /// annotations are emitted during lowering to the LLVM code.
+  /// Add global annotations for a global value.
+  /// Those annotations are emitted during lowering to the LLVM code.
   void addGlobalAnnotations(const ValueDecl *d, mlir::Operation *gv);
 };
 } // namespace cir

clang/lib/CIR/Dialect/Transforms/LoweringPrepare.cpp

@@ -888,7 +888,7 @@ void LoweringPreparePass::lowerGlobalOp(GlobalOp op) {
   }
 
   // Collect global annotations
-  auto annotations = op.getAnnotations();
+  std::optional<mlir::ArrayAttr> annotations = op.getAnnotations();
   if (annotations) {
     for (auto &annot : annotations.value()) {
       globalAnnotations.push_back({annot, op});
@@ -1084,9 +1084,9 @@ void LoweringPreparePass::buildGlobalAnnotationValues() {
   annotationValueVec.reserve(globalAnnotations.size());
 
   for (auto &annotEntry : globalAnnotations) {
-    auto annot = dyn_cast<mlir::cir::AnnotationAttr>(annotEntry.first);
+    auto annot = cast<mlir::cir::AnnotationAttr>(annotEntry.first);
     mlir::Operation *op = annotEntry.second;
-    auto globalValue = dyn_cast<mlir::SymbolOpInterface>(op);
+    auto globalValue = cast<mlir::SymbolOpInterface>(op);
     mlir::StringAttr globalValueName = globalValue.getNameAttr();
     SmallVector<mlir::Attribute, 2> entryArray = {globalValueName, annot};
     mlir::cir::GlobalAnnotationValueAttr valueEntry =
@@ -1133,7 +1133,7 @@ void LoweringPreparePass::runOnOp(Operation *op) {
     } else if (auto globalDtor = fnOp.getGlobalDtorAttr()) {
       globalDtorList.push_back(globalDtor);
     }
-    if (auto annotates = fnOp.getAnnotations()) {
+    if (std::optional<mlir::ArrayAttr> annotates = fnOp.getAnnotations()) {
       for (auto &annot : annotates.value()) {
         globalAnnotations.push_back({annot, fnOp});
       }

clang/test/CIR/IR/annotations.cir

@@ -0,0 +1,20 @@
+// RUN: cir-opt %s -o %t.cir
+// RUN: FileCheck --input-file=%t.cir %s
+
+!s32i = !cir.int<s, 32>
+module {
+cir.global external @a = #cir.int<0> : !s32i [#cir.annotation<name = "testanno", args = ["21", 12 : i32]>]
+cir.func @foo() attributes {annotations = [#cir.annotation<name = "withargfunc", args = ["os", 22 : i32]>]} {
+    cir.return
+}
+cir.func @bar() attributes {annotations = [#cir.annotation<name = "noargfunc", args = []>, #cir.annotation<name = "withargfunc", args = ["os", 23 : i32]>]} {
+    cir.return
+}
+}
+// CHECK: cir.global  external @a = #cir.int<0> : !s32i
+// CHECK-SAME: [#cir.annotation<name = "testanno", args = ["21", 12 : i32]>]
+// CHECK: cir.func @foo() attributes
+// CHECK-SAME: {annotations = [#cir.annotation<name = "withargfunc", args = ["os", 22 : i32]>]}
+// CHECK: cir.func @bar() attributes
+// CHECK-SAME: {annotations = [#cir.annotation<name = "noargfunc", args = []>,
+// CHECK-SAME: #cir.annotation<name = "withargfunc", args = ["os", 23 : i32]>]}