Skip to content

[CIR] Make LLVM & OGCG variables match the same pattern #135427

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Apr 11, 2025
Merged

[CIR] Make LLVM & OGCG variables match the same pattern #135427

merged 1 commit into from
Apr 11, 2025

Conversation

AmrDeveloper
Copy link
Member

Follow-up patch to improve variable names in LLVM and OGCG in #134536

@AmrDeveloper AmrDeveloper added the ClangIR Anything related to the ClangIR project label Apr 11, 2025
@llvmbot llvmbot added the clang Clang issues not falling into any other category label Apr 11, 2025
@llvmbot
Copy link
Member

llvmbot commented Apr 11, 2025 •
edited
Loading

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clangir

Author: Amr Hesham (AmrDeveloper)

Changes

Follow-up patch to improve variable names in LLVM and OGCG in #134536

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

1 Files Affected:

  • (modified) clang/test/CIR/CodeGen/array.cpp (+55-54)
diff --git a/clang/test/CIR/CodeGen/array.cpp b/clang/test/CIR/CodeGen/array.cpp
index 431164c797f76..5cda061cdbf12 100644
--- a/clang/test/CIR/CodeGen/array.cpp
+++ b/clang/test/CIR/CodeGen/array.cpp
@@ -94,26 +94,27 @@ void func() {
 // CIR" cir.store %[[TMP]], %[[INIT_2]] : !s32i, !cir.ptr<!s32i>
 
 // LLVM: define void @func()
-// LLVM-NEXT: %[[ARR_ALLOCA:.*]] = alloca [10 x i32], i64 1, align 16
+// LLVM-NEXT: %[[ARR:.*]] = alloca [10 x i32], i64 1, align 16
 // LLVM-NEXT: %[[INIT:.*]] = alloca i32, i64 1, align 4
 // LLVM-NEXT: %[[INIT_2:.*]] = alloca i32, i64 1, align 4
-// LLVM-NEXT: %[[ARR_PTR:.*]] = getelementptr i32, ptr %[[ARR_ALLOCA]], i32 0
+// LLVM-NEXT: %[[ARR_PTR:.*]] = getelementptr i32, ptr %[[ARR]], i32 0
 // LLVM-NEXT: %[[ELE_PTR:.*]] = getelementptr i32, ptr %[[ARR_PTR]], i64 0
-// LLVM-NEXT: %[[TMP:.*]] = load i32, ptr %[[ELE_PTR]], align 4
-// LLVM-NEXT: store i32 %[[TMP]], ptr %[[INIT]], align 4
-// LLVM-NEXT: %[[ARR_PTR:.*]] = getelementptr i32, ptr %[[ARR_ALLOCA]], i32 0
+// LLVM-NEXT: %[[TMP_1:.*]] = load i32, ptr %[[ELE_PTR]], align 4
+// LLVM-NEXT: store i32 %[[TMP_1]], ptr %[[INIT]], align 4
+// LLVM-NEXT: %[[ARR_PTR:.*]] = getelementptr i32, ptr %[[ARR]], i32 0
 // LLVM-NEXT: %[[ELE_PTR:.*]] = getelementptr i32, ptr %[[ARR_PTR]], i64 1
-// LLVM-NEXT: %[[TMP:.*]] = load i32, ptr %[[ELE_PTR]], align 4
-
-// OGCG: %arr = alloca [10 x i32], align 16
-// OGCG: %e = alloca i32, align 4
-// OGCG: %e2 = alloca i32, align 4
-// OGCG: %arrayidx = getelementptr inbounds [10 x i32], ptr %arr, i64 0, i64 0
-// OGCG: %0 = load i32, ptr %arrayidx, align 16
-// OGCG: store i32 %0, ptr %e, align 4
-// OGCG: %arrayidx1 = getelementptr inbounds [10 x i32], ptr %arr, i64 0, i64 1
-// OGCG: %1 = load i32, ptr %arrayidx1, align 4
-// OGCG: store i32 %1, ptr %e2, align 4
+// LLVM-NEXT: %[[TMP_2:.*]] = load i32, ptr %[[ELE_PTR]], align 4
+// LLVM-NEXT: store i32 %[[TMP_2]], ptr %[[INIT_2]], align 4
+
+// OGCG: %[[ARR:.*]] = alloca [10 x i32], align 16
+// OGCG: %[[INIT:.*]] = alloca i32, align 4
+// OGCG: %[[INIT_2:.*]] = alloca i32, align 4
+// OGCG: %[[ELE_PTR:.*]] = getelementptr inbounds [10 x i32], ptr %[[ARR]], i64 0, i64 0
+// OGCG: %[[TMP_1:.*]] = load i32, ptr %[[ELE_PTR]], align 16
+// OGCG: store i32 %[[TMP_1]], ptr %[[INIT]], align 4
+// OGCG: %[[ELE_PTR:.*]] = getelementptr inbounds [10 x i32], ptr %[[ARR]], i64 0, i64 1
+// OGCG: %[[TMP_2:.*]] = load i32, ptr %[[ELE_PTR]], align 4
+// OGCG: store i32 %[[TMP_2]], ptr %[[INIT_2]], align 4
 
 void func2() {
   int arr[2] = {5};
@@ -135,9 +136,9 @@ void func2() {
 // CIR: cir.store %[[ELE_1_PTR]], %[[ELE_ALLOCA]] : !cir.ptr<!s32i>, !cir.ptr<!cir.ptr<!s32i>>
 
 // LLVM: define void @func2()
-// LLVM:  %[[ARR_ALLOCA:.*]] = alloca [2 x i32], i64 1, align 4
+// LLVM:  %[[ARR:.*]] = alloca [2 x i32], i64 1, align 4
 // LLVM:  %[[TMP:.*]] = alloca ptr, i64 1, align 8
-// LLVM:  %[[ARR_PTR:.*]] = getelementptr i32, ptr %[[ARR_ALLOCA]], i32 0
+// LLVM:  %[[ARR_PTR:.*]] = getelementptr i32, ptr %[[ARR]], i32 0
 // LLVM:  store i32 5, ptr %[[ARR_PTR]], align 4
 // LLVM:  %[[ELE_1_PTR:.*]] = getelementptr i32, ptr %[[ARR_PTR]], i64 1
 // LLVM:  store ptr %[[ELE_1_PTR]], ptr %[[TMP]], align 8
@@ -146,8 +147,8 @@ void func2() {
 // LLVM:  %[[ELE_1:.*]] = getelementptr i32, ptr %[[TMP2]], i64 1
 // LLVM:  store ptr %[[ELE_1]], ptr %[[TMP]], align 8
 
-// OGCG: %arr = alloca [2 x i32], align 4
-// OGCG: call void @llvm.memcpy.p0.p0.i64(ptr align 4 %arr, ptr align 4 @[[FUN2_ARR]], i64 8, i1 false)
+// OGCG: %[[ARR:.*]] = alloca [2 x i32], align 4
+// OGCG: call void @llvm.memcpy.p0.p0.i64(ptr align 4 %[[ARR]], ptr align 4 @[[FUN2_ARR]], i64 8, i1 false)
 
 void func3() {
   int arr[2] = {5, 6};
@@ -175,31 +176,31 @@ void func3() {
 // CIR: cir.store %[[ELE_TMP]], %[[INIT]] : !s32i, !cir.ptr<!s32i>
 
 // LLVM: define void @func3()
-// LLVM:  %[[ARR_ALLOCA:.*]] = alloca [2 x i32], i64 1, align 4
+// LLVM:  %[[ARR:.*]] = alloca [2 x i32], i64 1, align 4
 // LLVM:  %[[IDX:.*]] = alloca i32, i64 1, align 4
 // LLVM:  %[[INIT:.*]] = alloca i32, i64 1, align 4
-// LLVM:  %[[ARR_PTR:.*]] = getelementptr i32, ptr %[[ARR_ALLOCA]], i32 0
+// LLVM:  %[[ARR_PTR:.*]] = getelementptr i32, ptr %[[ARR]], i32 0
 // LLVM:  store i32 5, ptr %[[ARR_PTR]], align 4
 // LLVM:  %[[ELE_1_PTR:.*]] = getelementptr i32, ptr %[[ARR_PTR]], i64 1
 // LLVM:  store i32 6, ptr %[[ELE_1_PTR]], align 4
 // LLVM:  store i32 1, ptr %[[IDX]], align 4
 // LLVM:  %[[TMP1:.*]] = load i32, ptr %[[IDX]], align 4
-// LLVM:  %[[ARR_PTR:.*]] = getelementptr i32, ptr %[[ARR_ALLOCA]], i32 0
+// LLVM:  %[[ARR_PTR:.*]] = getelementptr i32, ptr %[[ARR]], i32 0
 // LLVM:  %[[IDX_I64:.*]] = sext i32 %[[TMP1]] to i64
 // LLVM:  %[[ELE:.*]] = getelementptr i32, ptr %[[ARR_PTR]], i64 %[[IDX_I64]]
 // LLVM:  %[[TMP2:.*]] = load i32, ptr %[[ELE]], align 4
 // LLVM:  store i32 %[[TMP2]], ptr %[[INIT]], align 4
 
-// OGCG:  %arr = alloca [2 x i32], align 4
-// OGCG:  %idx = alloca i32, align 4
-// OGCG:  %e = alloca i32, align 4
-// OGCG:  call void @llvm.memcpy.p0.p0.i64(ptr align 4 %arr, ptr align 4 @[[FUN3_ARR]], i64 8, i1 false)
-// OGCG:  store i32 1, ptr %idx, align 4
-// OGCG:  %0 = load i32, ptr %idx, align 4
-// OGCG:  %idxprom = sext i32 %0 to i64
-// OGCG:  %arrayidx = getelementptr inbounds [2 x i32], ptr %arr, i64 0, i64 %idxprom
-// OGCG:  %1 = load i32, ptr %arrayidx, align 4
-// OGCG:  store i32 %1, ptr %e, align 4
+// OGCG:  %[[ARR:.*]] = alloca [2 x i32], align 4
+// OGCG:  %[[IDX:.*]] = alloca i32, align 4
+// OGCG:  %[[INIT:.*]] = alloca i32, align 4
+// OGCG:  call void @llvm.memcpy.p0.p0.i64(ptr align 4 %[[ARR]], ptr align 4 @[[FUN3_ARR]], i64 8, i1 false)
+// OGCG:  store i32 1, ptr %[[IDX]], align 4
+// OGCG:  %[[TMP:.*]] = load i32, ptr %[[IDX]], align 4
+// OGCG:  %[[IDX_I64:.*]] = sext i32 %[[TMP]] to i64
+// OGCG:  %[[ELE:.*]] = getelementptr inbounds [2 x i32], ptr %[[ARR]], i64 0, i64 %[[IDX_I64]]
+// OGCG:  %[[TMP_2:.*]] = load i32, ptr %[[ELE]], align 4
+// OGCG:  store i32 %[[TMP_2:.*]], ptr %[[INIT]], align 4
 
 void func4() {
   int arr[2][1] = {{5}, {6}};
@@ -227,28 +228,28 @@ void func4() {
 // CIR: cir.store %[[TMP]], %[[INIT]] : !s32i, !cir.ptr<!s32i>
 
 // LLVM: define void @func4()
-// LLVM:  %[[ARR_ALLOCA:.*]] = alloca [2 x [1 x i32]], i64 1, align 4
+// LLVM:  %[[ARR:.*]] = alloca [2 x [1 x i32]], i64 1, align 4
 // LLVM:  %[[INIT:.*]] = alloca i32, i64 1, align 4
-// LLVM:  %[[ARR_PTR:.*]] = getelementptr [1 x i32], ptr %[[ARR_ALLOCA]], i32 0
+// LLVM:  %[[ARR_PTR:.*]] = getelementptr [1 x i32], ptr %[[ARR]], i32 0
 // LLVM:  %[[ARR_0_0:.*]] = getelementptr i32, ptr %[[ARR_PTR]], i32 0
 // LLVM:  store i32 5, ptr %[[ARR_0_0]], align 4
 // LLVM:  %[[ARR_1:.*]] = getelementptr [1 x i32], ptr %[[ARR_PTR]], i64 1
 // LLVM:  %[[ARR_1_0:.*]] = getelementptr i32, ptr %[[ARR_1]], i32 0
 // LLVM:  store i32 6, ptr %[[ARR_1_0]], align 4
-// LLVM:  %[[ARR_PTR:.*]] = getelementptr [1 x i32], ptr %[[ARR_ALLOCA]], i32 0
+// LLVM:  %[[ARR_PTR:.*]] = getelementptr [1 x i32], ptr %[[ARR]], i32 0
 // LLVM:  %[[ARR_1:.*]] = getelementptr [1 x i32], ptr %[[ARR_PTR]], i64 1
 // LLVM:  %[[ARR_1_0:.*]] = getelementptr i32, ptr %[[ARR_1]], i32 0
 // LLVM:  %[[ELE_PTR:.*]] = getelementptr i32, ptr %[[ARR_1_0]], i64 0
 // LLVM:  %[[TMP:.*]] = load i32, ptr %[[ELE_PTR]], align 4
 // LLVM:  store i32 %[[TMP]], ptr %[[INIT]], align 4
 
-// OGCG: %arr = alloca [2 x [1 x i32]], align 4
-// OGCG: %e = alloca i32, align 4
-// OGCG: call void @llvm.memcpy.p0.p0.i64(ptr align 4 %arr, ptr align 4 @[[FUN4_ARR]], i64 8, i1 false)
-// OGCG: %arrayidx = getelementptr inbounds [2 x [1 x i32]], ptr %arr, i64 0, i64 1
-// OGCG: %arrayidx1 = getelementptr inbounds [1 x i32], ptr %arrayidx, i64 0, i64 0
-// OGCG: %0 = load i32, ptr %arrayidx1, align 4
-// OGCG: store i32 %0, ptr %e, align 4
+// OGCG: %[[ARR:.*]] = alloca [2 x [1 x i32]], align 4
+// OGCG: %[[INIT:.*]] = alloca i32, align 4
+// OGCG: call void @llvm.memcpy.p0.p0.i64(ptr align 4 %[[ARR]], ptr align 4 @[[FUN4_ARR]], i64 8, i1 false)
+// OGCG: %[[ARR_1:.*]] = getelementptr inbounds [2 x [1 x i32]], ptr %[[ARR]], i64 0, i64 1
+// OGCG: %[[ARR_1_0:.*]] = getelementptr inbounds [1 x i32], ptr %[[ARR_1]], i64 0, i64 0
+// OGCG: %[[TMP:.*]] = load i32, ptr %[[ARR_1_0]], align 4
+// OGCG: store i32 %[[TMP]], ptr %[[INIT]], align 4
 
 void func5() {
   int arr[2][1] = {{5}};
@@ -271,9 +272,9 @@ void func5() {
 // CIR: cir.store %10, %[[ARR_PTR]] : !cir.ptr<!cir.array<!s32i x 1>>, !cir.ptr<!cir.ptr<!cir.array<!s32i x 1>>>
 
 // LLVM: define void @func5()
-// LLVM:  %[[ARR_ALLOCA:.*]] = alloca [2 x [1 x i32]], i64 1, align 4
+// LLVM:  %[[ARR:.*]] = alloca [2 x [1 x i32]], i64 1, align 4
 // LLVM:  %[[TMP:.*]] = alloca ptr, i64 1, align 8
-// LLVM:  %[[ARR_PTR:.*]] = getelementptr [1 x i32], ptr %[[ARR_ALLOCA]], i32 0
+// LLVM:  %[[ARR_PTR:.*]] = getelementptr [1 x i32], ptr %[[ARR]], i32 0
 // LLVM:  %[[ARR_0:.*]] = getelementptr i32, ptr %[[ARR_PTR]], i32 0
 // LLVM:  store i32 5, ptr %[[ARR_0]], align 4
 // LLVM:  %[[ARR_1:.*]] = getelementptr [1 x i32], ptr %[[ARR_PTR]], i64 1
@@ -283,8 +284,8 @@ void func5() {
 // LLVM:  %[[ARR_1_PTR:.*]] = getelementptr [1 x i32], ptr %[[ARR_1_VAL]], i64 1
 // LLVM:  store ptr %[[ARR_1_PTR]], ptr %[[TMP]], align 8
 
-// ORGC: %arr = alloca [2 x [1 x i32]], align 4
-// ORGC: call void @llvm.memcpy.p0.p0.i64(ptr align 4 %arr, ptr align 4 @[[FUN5_ARR]], i64 8, i1 false)
+// ORGC: %[[ARR:.*]] = alloca [2 x [1 x i32]], align 4
+// ORGC: call void @llvm.memcpy.p0.p0.i64(ptr align 4 %[[ARR]], ptr align 4 @[[FUN5_ARR]], i64 8, i1 false)
 
 void func6() {
   int x = 4;
@@ -313,13 +314,13 @@ void func6() {
 // LLVM:  %[[ELE_1:.*]] = getelementptr i32, ptr %[[ELE_0]], i64 1
 // LLVM:  store i32 5, ptr %[[ELE_1]], align 4
 
-// OGCG:  %x = alloca i32, align 4
-// OGCG:  %arr = alloca [2 x i32], align 4
-// OGCG:  store i32 4, ptr %x, align 4
-// OGCG:  %0 = load i32, ptr %x, align 4
-// OGCG:  store i32 %0, ptr %arr, align 4
-// OGCG:  %arrayinit.element = getelementptr inbounds i32, ptr %arr, i64 1
-// OGCG:  store i32 5, ptr %arrayinit.element, align 4
+// OGCG:  %[[VAR:.*]] = alloca i32, align 4
+// OGCG:  %[[ARR:.*]] = alloca [2 x i32], align 4
+// OGCG:  store i32 4, ptr %[[VAR]], align 4
+// OGCG:  %[[ELE_0:.*]] = load i32, ptr %[[VAR]], align 4
+// OGCG:  store i32 %[[ELE_0]], ptr %[[ARR]], align 4
+// OGCG:  %[[ELE_1:.*]] = getelementptr inbounds i32, ptr %[[ARR]], i64 1
+// OGCG:  store i32 5, ptr %[[ELE_1:.*]], align 4
 
 void func7() {
   int* arr[1] = {};

andykaylor
andykaylor approved these changes Apr 11, 2025
View reviewed changes
Copy link
Contributor

@andykaylor andykaylor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To clarify I wasn't saying that the LLVM checks and OGCG checks needed to use the same patterns/names (though it's nice that they do now). The important thing was not to have the value identifiers hard-corded in the checks.

Thanks for the update!

@AmrDeveloper
Copy link
Member Author

To clarify I wasn't saying that the LLVM checks and OGCG checks needed to use the same patterns/names (though it's nice that they do now). The important thing was not to have the value identifiers hard-corded in the checks.

Thanks for the update!

I found that it easier to understand both if they are named with the same pattern, for example in the 2D Array index expression it's more clear now :D

Thank you

bcardosolopes
bcardosolopes approved these changes Apr 11, 2025
View reviewed changes
Copy link
Member

@bcardosolopes bcardosolopes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@andykaylor andykaylor merged commit 93370c4 into llvm:main Apr 11, 2025
14 checks passed
bcardosolopes added a commit to bcardosolopes/llvm-project that referenced this pull request Apr 11, 2025
@bcardosolopes
Merge remote-tracking branch 'origin/main' into mlir-llvm-indirect-br
5bea802 
* origin/main: (287 commits)
  [Sema] On Windows, silence erroneous warning when building with MSVC
  [lldb][lldbp-dap] On Windoows, silence warnings when building with MSVC
  [lldb] Fix erroneous return value
  [compiler-rt] On Windows, silence warning when building with Clang ToT
  [clang][unittests] On Windows, silence warning when building with MSVC
  [lldb] On Windows, silence warning when building with Clang ToT
  [CIR] Make LLVM & OGCG variables match the same pattern (llvm#135427)
  [mlir][SMT] upstream `SMT` dialect (llvm#131480)
  [clang] fix serialization for SubstNonTypeTemplateParmPackExpr (llvm#135428)
  [flang][openacc] Allow if_present multiple times on host_data and update (llvm#135422)
  [flang][openacc] Allow finalize clause on exit data more than once (llvm#135415)
  [flang] IEEE_SCALB and SCALE - kind=2, kind=3 (llvm#135374)
  [-Wunsafe-buffer-usage] Add findUnsafePointers (llvm#135421)
  [compiler-rt][sanitizer] add Haiku support (llvm#134772)
  [cpp23] Remove usage of std::aligned_union<> in llvm (llvm#135146)
  [mlir][tosa] Add error_if checks for Mul Op (llvm#135075)
  [VPlan] Merge cases using getResultType in inferScalarType (NFC).
  [flang][runtime] Fix recently broken big-endian formatted integer input (llvm#135417)
  [AMDGPU][Verifier] Mark calls to entry functions as invalid in the IR verifier (llvm#134910)
  [llvm][Hexagon] Promote operand v2i1 to v2i32 (llvm#135409)
  ...
var-const pushed a commit to ldionne/llvm-project that referenced this pull request Apr 17, 2025
@AmrDeveloper @var-const
[CIR] Make LLVM & OGCG variables match the same pattern (llvm#135427)
0703094 
Follow-up patch to improve variable names in LLVM and OGCG in
llvm#134536
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bcardosolopes bcardosolopes bcardosolopes approved these changes

@andykaylor andykaylor andykaylor approved these changes

@erichkeane erichkeane Awaiting requested review from erichkeane

@lanza lanza Awaiting requested review from lanza

Assignees
No one assigned
Labels
clang Clang issues not falling into any other category ClangIR Anything related to the ClangIR project
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@AmrDeveloper @llvmbot @bcardosolopes @andykaylor