[CIR] Introduce type aliases for records #136387

andykaylor · 2025-04-18T23:27:52Z

This introduces MLIR aliases for ClangIR record types. These are used in the incubator and having skipped over them upstream is causing the tests to diverge.

llvmbot · 2025-04-18T23:28:25Z

@llvm/pr-subscribers-clangir

@llvm/pr-subscribers-clang

Author: Andy Kaylor (andykaylor)

Changes

This introduces MLIR aliases for ClangIR record types. These are used in the incubator and having skipped over them upstream is causing the tests to diverge.

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

6 Files Affected:

(modified) clang/lib/CIR/Dialect/IR/CIRDialect.cpp (+8)
(modified) clang/test/CIR/CodeGen/struct.c (+15-16)
(modified) clang/test/CIR/CodeGen/struct.cpp (+2-3)
(modified) clang/test/CIR/CodeGen/typedef.c (+1-3)
(modified) clang/test/CIR/CodeGen/union.c (+2-3)
(modified) clang/test/CIR/IR/struct.cir (+10-4)

diff --git a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp
index d2313e75870b4..3e10d16886eb2 100644
--- a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp
+++ b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp
@@ -33,6 +33,14 @@ struct CIROpAsmDialectInterface : public OpAsmDialectInterface {
   using OpAsmDialectInterface::OpAsmDialectInterface;
 
   AliasResult getAlias(Type type, raw_ostream &os) const final {
+    if (auto recordType = dyn_cast<cir::RecordType>(type)) {
+      StringAttr nameAttr = recordType.getName();
+      if (!nameAttr)
+        os << "ty_anon_" << recordType.getKindAsStr();
+      else
+        os << "ty_" << nameAttr.getValue();
+      return AliasResult::OverridableAlias;
+    }
     if (auto intType = dyn_cast<cir::IntType>(type)) {
       // We only provide alias for standard integer types (i.e. integer types
       // whose width is a power of 2 and at least 8).
diff --git a/clang/test/CIR/CodeGen/struct.c b/clang/test/CIR/CodeGen/struct.c
index 3dc1655e15d2c..e75a2064eca4d 100644
--- a/clang/test/CIR/CodeGen/struct.c
+++ b/clang/test/CIR/CodeGen/struct.c
@@ -7,11 +7,19 @@
 
 // For LLVM IR checks, the structs are defined before the variables, so these
 // checks are at the top.
+// CIR-DAG: !ty_IncompleteS = !cir.record<struct "IncompleteS" incomplete>
+// CIR-DAG: !ty_CompleteS = !cir.record<struct "CompleteS" {!s32i, !s8i}>
+// CIR-DAG: !ty_OuterS = !cir.record<struct "OuterS" {!ty_InnerS, !s32i}>  
+// CIR-DAG: !ty_InnerS = !cir.record<struct "InnerS" {!s32i, !s8i}>
+// CIR-DAG: !ty_PackedS = !cir.record<struct "PackedS" packed {!s32i, !s8i}>
+// CIR-DAG: !ty_PackedAndPaddedS = !cir.record<struct "PackedAndPaddedS" packed padded {!s32i, !s8i, !u8i}>
+
 // LLVM: %struct.CompleteS = type { i32, i8 }
 // LLVM: %struct.OuterS = type { %struct.InnerS, i32 }
 // LLVM: %struct.InnerS = type { i32, i8 }
 // LLVM: %struct.PackedS = type <{ i32, i8 }>
 // LLVM: %struct.PackedAndPaddedS = type <{ i32, i8, i8 }>
+
 // OGCG: %struct.CompleteS = type { i32, i8 }
 // OGCG: %struct.OuterS = type { %struct.InnerS, i32 }
 // OGCG: %struct.InnerS = type { i32, i8 }
@@ -20,8 +28,7 @@
 
 struct IncompleteS *p;
 
-// CIR:      cir.global external @p = #cir.ptr<null> : !cir.ptr<!cir.record<struct
-// CIR-SAME:     "IncompleteS" incomplete>>
+// CIR:      cir.global external @p = #cir.ptr<null> : !cir.ptr<!ty_IncompleteS>
 // LLVM: @p = dso_local global ptr null
 // OGCG: @p = global ptr null, align 8
 
@@ -30,8 +37,7 @@ struct CompleteS {
   char b;
 } cs;
 
-// CIR:       cir.global external @cs = #cir.zero : !cir.record<struct
-// CIR-SAME:      "CompleteS" {!s32i, !s8i}>
+// CIR:       cir.global external @cs = #cir.zero : !ty_CompleteS
 // LLVM:      @cs = dso_local global %struct.CompleteS zeroinitializer
 // OGCG:      @cs = global %struct.CompleteS zeroinitializer, align 4
 
@@ -47,8 +53,7 @@ struct OuterS {
 
 struct OuterS os;
 
-// CIR:       cir.global external @os = #cir.zero : !cir.record<struct
-// CIR-SAME:      "OuterS" {!cir.record<struct "InnerS" {!s32i, !s8i}>, !s32i}>
+// CIR:       cir.global external @os = #cir.zero : !ty_OuterS
 // LLVM:      @os = dso_local global %struct.OuterS zeroinitializer
 // OGCG:      @os = global %struct.OuterS zeroinitializer, align 4
 
@@ -60,8 +65,7 @@ struct PackedS {
   char a1;
 } ps;
 
-// CIR:       cir.global external @ps = #cir.zero : !cir.record<struct "PackedS"
-// CIR-SAME:      packed {!s32i, !s8i}>
+// CIR:       cir.global external @ps = #cir.zero : !ty_PackedS
 // LLVM:      @ps = dso_local global %struct.PackedS zeroinitializer
 // OGCG:      @ps = global %struct.PackedS zeroinitializer, align 1
 
@@ -70,8 +74,7 @@ struct PackedAndPaddedS {
   char b1;
 } __attribute__((aligned(2))) pps;
 
-// CIR:       cir.global external @pps = #cir.zero : !cir.record<struct
-// CIR-SAME:      "PackedAndPaddedS" packed padded {!s32i, !s8i, !u8i}>
+// CIR:       cir.global external @pps = #cir.zero : !ty_PackedAndPaddedS
 // LLVM:      @pps = dso_local global %struct.PackedAndPaddedS zeroinitializer
 // OGCG:      @pps = global %struct.PackedAndPaddedS zeroinitializer, align 2
 
@@ -82,9 +85,7 @@ void f(void) {
 }
 
 // CIR:      cir.func @f()
-// CIR-NEXT:   cir.alloca !cir.ptr<!cir.record<struct "IncompleteS" incomplete>>,
-// CIR-SAME:       !cir.ptr<!cir.ptr<!cir.record<struct
-// CIR-SAME:       "IncompleteS" incomplete>>>, ["p"]
+// CIR-NEXT:   cir.alloca !cir.ptr<!ty_IncompleteS>, !cir.ptr<!cir.ptr<!ty_IncompleteS>>, ["p"] {alignment = 8 : i64}
 // CIR-NEXT:   cir.return
 
 // LLVM:      define void @f()
@@ -101,9 +102,7 @@ void f2(void) {
 }
 
 // CIR:      cir.func @f2()
-// CIR-NEXT:   cir.alloca !cir.record<struct "CompleteS" {!s32i, !s8i}>,
-// CIR-SAME:       !cir.ptr<!cir.record<struct "CompleteS" {!s32i, !s8i}>>,
-// CIR-SAME:       ["s"] {alignment = 4 : i64}
+// CIR-NEXT:   cir.alloca !ty_CompleteS, !cir.ptr<!ty_CompleteS>, ["s"] {alignment = 4 : i64}
 // CIR-NEXT:   cir.return
 
 // LLVM:      define void @f2()
diff --git a/clang/test/CIR/CodeGen/struct.cpp b/clang/test/CIR/CodeGen/struct.cpp
index 6197340a7d36b..71f8eef43f054 100644
--- a/clang/test/CIR/CodeGen/struct.cpp
+++ b/clang/test/CIR/CodeGen/struct.cpp
@@ -8,7 +8,7 @@
 struct IncompleteS;
 IncompleteS *p;
 
-// CIR: cir.global external @p = #cir.ptr<null> : !cir.ptr<!cir.record<struct "IncompleteS" incomplete>>
+// CIR: cir.global external @p = #cir.ptr<null> : !cir.ptr<!ty_IncompleteS>
 // LLVM: @p = dso_local global ptr null
 // OGCG: @p = global ptr null, align 8
 
@@ -17,8 +17,7 @@ void f(void) {
 }
 
 // CIR:      cir.func @f()
-// CIR-NEXT:   cir.alloca !cir.ptr<!cir.record<struct "IncompleteS" incomplete>>,
-// CIR-SAME:       !cir.ptr<!cir.ptr<!cir.record<struct "IncompleteS" incomplete>>>, ["p"]
+// CIR-NEXT:   cir.alloca !cir.ptr<!ty_IncompleteS>, !cir.ptr<!cir.ptr<!ty_IncompleteS>>, ["p"]
 // CIR-NEXT:   cir.return
 
 // LLVM:      define void @f()
diff --git a/clang/test/CIR/CodeGen/typedef.c b/clang/test/CIR/CodeGen/typedef.c
index 17fce13abf38a..ad0000730e311 100644
--- a/clang/test/CIR/CodeGen/typedef.c
+++ b/clang/test/CIR/CodeGen/typedef.c
@@ -11,9 +11,7 @@ void local_typedef(void) {
 }
 
 // CIR:      cir.func @local_typedef()
-// CIR:        cir.alloca !cir.record<struct "Struct" {!s32i}>,
-// CIR-SAME:       !cir.ptr<!cir.record<struct "Struct" {!s32i}>>, ["s"]
-// CIR-SAME:       {alignment = 4 : i64}
+// CIR:        cir.alloca !ty_Struct, !cir.ptr<!ty_Struct>, ["s"] {alignment = 4 : i64}
 // CIR:        cir.return
 
 // LLVM: %struct.Struct = type { i32 }
diff --git a/clang/test/CIR/CodeGen/union.c b/clang/test/CIR/CodeGen/union.c
index 075d0d2315508..c9b8e7e9e2908 100644
--- a/clang/test/CIR/CodeGen/union.c
+++ b/clang/test/CIR/CodeGen/union.c
@@ -7,7 +7,7 @@
 
 union IncompleteU *p;
 
-// CIR: cir.global external @p = #cir.ptr<null> : !cir.ptr<!cir.record<union "IncompleteU" incomplete>>
+// CIR: cir.global external @p = #cir.ptr<null> : !cir.ptr<!ty_IncompleteU>
 // LLVM: @p = dso_local global ptr null
 // OGCG: @p = global ptr null, align 8
 
@@ -16,8 +16,7 @@ void f(void) {
 }
 
 // CIR: cir.func @f()
-// CIR-NEXT: cir.alloca !cir.ptr<!cir.record<union "IncompleteU" incomplete>>,
-// CIR-SAME:     !cir.ptr<!cir.ptr<!cir.record<union "IncompleteU" incomplete>>>, ["p"]
+// CIR-NEXT: cir.alloca !cir.ptr<!ty_IncompleteU>, !cir.ptr<!cir.ptr<!ty_IncompleteU>>, ["p"]
 // CIR-NEXT: cir.return
 
 // LLVM:      define void @f()
diff --git a/clang/test/CIR/IR/struct.cir b/clang/test/CIR/IR/struct.cir
index b6ed1d78b354a..ba9c21890e3ed 100644
--- a/clang/test/CIR/IR/struct.cir
+++ b/clang/test/CIR/IR/struct.cir
@@ -1,9 +1,15 @@
 // RUN: cir-opt %s | FileCheck %s
 
+!ty_S = !cir.record<struct "S" incomplete>
+!ty_U = !cir.record<union "U" incomplete>
+
+// CHECK: !ty_S = !cir.record<struct "S" incomplete>
+// CHECK: !ty_U = !cir.record<union "U" incomplete>
+
 module  {
-    cir.global external @p1 = #cir.ptr<null> : !cir.ptr<!cir.record<struct "S" incomplete>>
-    cir.global external @p2 = #cir.ptr<null> : !cir.ptr<!cir.record<union "U" incomplete>>
+    cir.global external @p1 = #cir.ptr<null> : !cir.ptr<!ty_S>
+    cir.global external @p2 = #cir.ptr<null> : !cir.ptr<!ty_U>
 }
 
-// CHECK: cir.global external @p1 = #cir.ptr<null> : !cir.ptr<!cir.record<struct "S" incomplete>>
-// CHECK: cir.global external @p2 = #cir.ptr<null> : !cir.ptr<!cir.record<union "U" incomplete>>
+// CHECK: cir.global external @p1 = #cir.ptr<null> : !cir.ptr<!ty_S>
+// CHECK: cir.global external @p2 = #cir.ptr<null> : !cir.ptr<!ty_U>

erichkeane

Seems reasonable to me, but not the best one to review this.

I question stealing ty as the prefix to this instead of rt (or something more specific to records), but I'm not sure I feel particularly strongly. Just it seems that ty is going to be pretty meaningless perhaps?

ALSO, I see no tests that contain ty_anon, can you write some?

Finally: Why the prefix at all?

bcardosolopes · 2025-04-21T17:55:49Z

I question stealing ty as the prefix to this instead of rt (or something more specific to records), but I'm not sure I feel particularly strongly. Just it seems that ty is going to be pretty meaningless perhaps?

Good point, rec or rt probably make more sense. I'm fine if this change comes in the future in case it's a lot of noise for the changes required on existing tests here and in the incubator.

bcardosolopes

LGTM pending ty_anon test mentioned by Erich

andykaylor · 2025-04-21T18:59:23Z

ALSO, I see no tests that contain ty_anon, can you write some?

We don't actually generate anon records (either upstream or in the incubator). I think we should just make them illegal. Anonymous structs in the source are given a name in CIR (anon.<n>)

bcardosolopes · 2025-04-22T18:08:44Z

I think we should just make them illegal.

sounds good to me

andykaylor · 2025-04-22T19:35:00Z

@erichkeane @bcardosolopes It turns out I was wrong in claiming that the incubator never generates anonymous records. It generates anonymous records for vtables and typeinfo. This is consistent with the LLVM IR generated by classic codegen, so maybe I shouldn't be getting rid of that after all. There's also a place where it looks like a temporary anonymous record is being generated to compare the record layout to another type in the constant aggregate builder.

erichkeane · 2025-04-22T19:45:35Z

@erichkeane @bcardosolopes It turns out I was wrong in claiming that the incubator never generates anonymous records. It generates anonymous records for vtables and typeinfo. This is consistent with the LLVM IR generated by classic codegen, so maybe I shouldn't be getting rid of that after all. There's also a place where it looks like a temporary anonymous record is being generated to compare the record layout to another type in the constant aggregate builder.

Ok, cool! Could we add these details as a comment somewhere to make sure I don't comment on this again in the future :) ?

This introduces MLIR aliases for ClangIR record types.

bcardosolopes · 2025-04-23T17:49:18Z

It generates anonymous records for vtables and typeinfo

oh right, that was it! thanks for double checking

This introduces MLIR aliases for ClangIR record types. These are used in the incubator and having skipped over them upstream is causing the tests to diverge.

andykaylor requested review from xlauko, bcardosolopes, erichkeane, lanza and AmrDeveloper April 18, 2025 23:27

llvmbot added clang Clang issues not falling into any other category ClangIR Anything related to the ClangIR project labels Apr 18, 2025

erichkeane approved these changes Apr 19, 2025

View reviewed changes

bcardosolopes approved these changes Apr 21, 2025

View reviewed changes

andykaylor mentioned this pull request Apr 21, 2025

[CIR] Change record type alias prefix from !ty_ to !rec_ llvm/clangir#1580

Merged

andykaylor added 3 commits April 22, 2025 17:04

[CIR] Introduce type aliases for records

f161bed

This introduces MLIR aliases for ClangIR record types.

Updated prefix to "rec_"

46d2706

Update struct.c test after rebase

c9b557b

andykaylor force-pushed the cir-struct-alias branch from a8543dc to c9b557b Compare April 23, 2025 00:18

andykaylor merged commit 0f5965f into llvm:main Apr 23, 2025
11 checks passed

andykaylor deleted the cir-struct-alias branch April 24, 2025 23:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[CIR] Introduce type aliases for records #136387

[CIR] Introduce type aliases for records #136387

Uh oh!

andykaylor commented Apr 18, 2025

Uh oh!

llvmbot commented Apr 18, 2025 •

edited

Loading

Uh oh!

erichkeane left a comment

Uh oh!

bcardosolopes commented Apr 21, 2025

Uh oh!

bcardosolopes left a comment

Uh oh!

andykaylor commented Apr 21, 2025

Uh oh!

bcardosolopes commented Apr 22, 2025

Uh oh!

andykaylor commented Apr 22, 2025

Uh oh!

erichkeane commented Apr 22, 2025

Uh oh!

bcardosolopes commented Apr 23, 2025

Uh oh!

Uh oh!

Uh oh!

[CIR] Introduce type aliases for records #136387

[CIR] Introduce type aliases for records #136387

Uh oh!

Conversation

andykaylor commented Apr 18, 2025

Uh oh!

llvmbot commented Apr 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

erichkeane left a comment

Choose a reason for hiding this comment

Uh oh!

bcardosolopes commented Apr 21, 2025

Uh oh!

bcardosolopes left a comment

Choose a reason for hiding this comment

Uh oh!

andykaylor commented Apr 21, 2025

Uh oh!

bcardosolopes commented Apr 22, 2025

Uh oh!

andykaylor commented Apr 22, 2025

Uh oh!

erichkeane commented Apr 22, 2025

Uh oh!

bcardosolopes commented Apr 23, 2025

Uh oh!

Uh oh!

Uh oh!

llvmbot commented Apr 18, 2025 •

edited

Loading