llvmbot wrote:

<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Artemiy (NotLebedev)

<details>
<summary>Changes</summary>

Closes #<!-- -->189666 .

Fix incorrect printing and parsing of `cir.global` if `global_visibility` 
attribute is present. Incorrect assembly format 
```
(`` $global_visibility^)?
```

Resulted in keyword sticking to previous word and producing incorrect cir like 
this:
```
cir.globalhidden external dso_local @<!-- -->hidden_var = #cir.int&lt;10&gt; : 
!s32i {alignment = 4 : i64} loc(#loc22)
cir.global "private"hidden internal dso_local @<!-- -->hidden_static_var = 
#cir.int&lt;10&gt; : !s32i {alignment = 4 : i64} loc(#loc24)
```

Using custom parser/printer that is used in `cir.func` parser fixes this issue 
and makes printed/parsed attribute for functions and global values consistent.

Also added tests for both global values and functions.

---
Full diff: https://github.com/llvm/llvm-project/pull/189673.diff


4 Files Affected:

- (modified) clang/include/clang/CIR/Dialect/IR/CIROps.td (+1-1) 
- (modified) clang/lib/CIR/Dialect/IR/CIRDialect.cpp (+7-1) 
- (added) clang/test/CIR/CodeGen/attribute-visibility.c (+45) 
- (added) clang/test/CIR/IR/attribute-visibility.cir (+28) 


``````````diff
diff --git a/clang/include/clang/CIR/Dialect/IR/CIROps.td 
b/clang/include/clang/CIR/Dialect/IR/CIROps.td
index b15dc30ffb87d..89652cb392bfb 100644
--- a/clang/include/clang/CIR/Dialect/IR/CIROps.td
+++ b/clang/include/clang/CIR/Dialect/IR/CIROps.td
@@ -2849,7 +2849,7 @@ def CIR_GlobalOp : CIR_Op<"global", [
 
   let assemblyFormat = [{
     ($sym_visibility^)?
-    (`` $global_visibility^)?
+    (custom<VisibilityAttr>($global_visibility)^)?
     (`constant` $constant^)?
     $linkage
     (`comdat` $comdat^)?
diff --git a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp 
b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp
index eb322d135a804..1187fd5ddea5d 100644
--- a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp
+++ b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp
@@ -221,10 +221,16 @@ void printVisibilityAttr(OpAsmPrinter &printer,
   }
 }
 
-void parseVisibilityAttr(OpAsmParser &parser, cir::VisibilityAttr &visibility) 
{
+void printVisibilityAttr(OpAsmPrinter &printer, cir::GlobalOp,
+                         cir::VisibilityAttr visibility) {
+    printVisibilityAttr(printer, visibility);
+}
+
+mlir::OptionalParseResult parseVisibilityAttr(OpAsmParser &parser, 
cir::VisibilityAttr &visibility) {
   cir::VisibilityKind visibilityKind =
       parseOptionalCIRKeyword(parser, cir::VisibilityKind::Default);
   visibility = cir::VisibilityAttr::get(parser.getContext(), visibilityKind);
+  return mlir::success();
 }
 
 
//===----------------------------------------------------------------------===//
diff --git a/clang/test/CIR/CodeGen/attribute-visibility.c 
b/clang/test/CIR/CodeGen/attribute-visibility.c
new file mode 100644
index 0000000000000..45098770d84ea
--- /dev/null
+++ b/clang/test/CIR/CodeGen/attribute-visibility.c
@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %s -o 
%t.cir
+// RUN: FileCheck --check-prefix=CIR --input-file=%t.cir %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-llvm %s -o 
%t-cir.ll
+// RUN: FileCheck --check-prefix=LLVM --input-file=%t-cir.ll %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm %s -o %t.ll
+// RUN: FileCheck --check-prefix=OGCG --input-file=%t.ll %s
+
+int normal_var = 10;
+// CIR: cir.global external @normal_var {{.*}}
+// LLVM: @normal_var = global {{.*}}
+// OGCG: @normal_var = global {{.*}}
+
+__attribute__((visibility("hidden")))
+int hidden_var = 10;
+// CIR: cir.global hidden external @hidden_var {{.*}}
+// LLVM: @hidden_var = hidden global {{.*}}
+// OGCG: @hidden_var = hidden global {{.*}}
+
+static int normal_static_var = 10;
+// CIR: cir.global "private" internal dso_local @normal_static_var {{.*}}
+// LLVM: @normal_static_var = internal global {{.*}}
+// OGCG: @normal_static_var = internal global {{.*}}
+
+__attribute__((visibility("hidden")))
+static int hidden_static_var = 10;
+// CIR: cir.global "private" hidden internal dso_local @hidden_static_var 
{{.*}}
+// LLVM: @hidden_static_var = internal hidden global {{.*}}
+// OGCG: @hidden_static_var = internal global {{.*}}
+
+void normal_func() {
+    normal_var = 0;
+    normal_static_var = 0;
+}
+// CIR: cir.func no_inline no_proto dso_local @normal_func() {{.*}} {
+// LLVM: define dso_local void @normal_func() {{.*}}
+// OGCG: define dso_local void @normal_func() {{.*}}
+
+__attribute__((visibility("hidden")))
+void hidden_func() {
+    hidden_var = 0;
+    hidden_static_var = 0;
+}
+// CIR: cir.func no_inline no_proto hidden dso_local @hidden_func() {{.*}} {
+// LLVM: define hidden void @hidden_func() {{.*}}
+// OGCG: define hidden void @hidden_func() {{.*}}
diff --git a/clang/test/CIR/IR/attribute-visibility.cir 
b/clang/test/CIR/IR/attribute-visibility.cir
new file mode 100644
index 0000000000000..e4f09a9d657cf
--- /dev/null
+++ b/clang/test/CIR/IR/attribute-visibility.cir
@@ -0,0 +1,28 @@
+// RUN: cir-opt %s --verify-roundtrip | FileCheck %s
+
+!s32i = !cir.int<s, 32>
+module {
+  cir.global external dso_local @normal_var = #cir.int<10> : !s32i {alignment 
= 4 : i64}
+  // CHECK: cir.global external dso_local @normal_var = #cir.int<10> : !s32i 
{alignment = 4 : i64}
+
+  cir.global hidden external dso_local @hidden_var = #cir.int<10> : !s32i 
{alignment = 4 : i64}
+  // CHECK: cir.global hidden external dso_local @hidden_var = #cir.int<10> : 
!s32i {alignment = 4 : i64}
+
+  cir.global "private" internal dso_local @normal_static_var = #cir.int<10> : 
!s32i {alignment = 4 : i64}
+  // CHECK: cir.global "private" internal dso_local @normal_static_var = 
#cir.int<10> : !s32i {alignment = 4 : i64}
+
+  cir.global "private" hidden internal dso_local @hidden_static_var = 
#cir.int<10> : !s32i {alignment = 4 : i64}
+  // CHECK: cir.global "private" hidden internal dso_local @hidden_static_var 
= #cir.int<10> : !s32i {alignment = 4 : i64}
+
+  cir.func no_inline no_proto dso_local @normal_func() {
+    cir.return
+  }
+  // CHECK: cir.func no_inline no_proto dso_local @normal_func()
+  // CHECK:   cir.return
+
+  cir.func no_inline no_proto hidden dso_local @hidden_func() {
+    cir.return
+  }
+  // CHECK: cir.func no_inline no_proto hidden dso_local @hidden_func()
+  // CHECK:   cir.return
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/189673
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to