Skip to content

Conversation

@Rajveer100
Copy link
Member

@Rajveer100 Rajveer100 commented Jan 13, 2025

Resolves #1266

After change:

%1 = alloca ptr, i64 1, align 8
  store i32 1, ptr @g_arr, align 4
  store i32 2, ptr getelementptr (i32, ptr @g_arr, i64 1), align 4
  store i32 3, ptr getelementptr (i32, ptr @g_arr, i64 2), align 4
  %2 = load i32, ptr @g, align 4
  store i32 %2, ptr getelementptr (i32, ptr @g_arr, i64 3), align 4
  store ptr getelementptr (i32, ptr getelementptr (i32, ptr @g_arr, i64 3), i64 1), ptr %1, align 8

@Rajveer100
Copy link
Member Author

This change should be done for the zero-fill as well I believe in the do while part.

@bcardosolopes
Copy link
Member

Thanks for working on this! Next step is to change/add testcases for you change, if you run ninja check-clang-cir you will find out what breaks. Because you are changing CIR code that we want to reflect in LLVM code, the testcase should check both IRs, see an example here: clang/test/CIR/CodeGen/abstract-cond.c

@bcardosolopes bcardosolopes changed the title [CIRCodeGen] Simplify LLVM IR array initialization for clang CIR [CIR][CIRGen] Simplify LLVM IR array initialization for clang CIR Jan 13, 2025
@Rajveer100
Copy link
Member Author

Rajveer100 commented Jan 14, 2025

Is there any automation tool for clang-cir file check, I remember using an update_* script?

// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %s -o %t.cir
// RUN: build/bin/FileCheck --check-prefix=CIR --input-file=%t.cir %s
// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-llvm -fno-clangir-call-conv-lowering %s -o %t.ll
// RUN: build/bin/FileCheck --check-prefix=LLVM --input-file=%t.ll %s

I have fixed the old broken test, for now adding the one from the original issue snippet:

void aggr_init() {
  int g = 5;
  int g_arr[5] = {1, 2, 3, g};
}

@bcardosolopes
Copy link
Member

You might be able to get something using mlir/utils/generate-test-checks.py, but not perfect.

// RUN: build/bin/FileCheck --check-prefix=CIR --input-file=%t.cir %s

Please do like the status quo:

// RUN: FileCheck --check-prefix=CIR --input-file=%t.cir %s

@Rajveer100
Copy link
Member Author

Done with the changes, let me know if anything else is needed.

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.

LGTM, will merge once tests pass!

Minor nit: mechanical changes (in this case CHECK to CIR, which is great btw) should come in their own PRs because it makes hard to spot the actual functionality change during review. It's fine for this time around cause of the previous review iteration, but just a heads up in case you end up sending more PRs.

@Rajveer100
Copy link
Member Author

Could you land this for me, the tests have passed?

@bcardosolopes bcardosolopes merged commit c695e79 into llvm:main Jan 21, 2025
6 checks passed
lanza pushed a commit that referenced this pull request Mar 18, 2025
)

Resolves #1266

After change:

```llvm
%1 = alloca ptr, i64 1, align 8
  store i32 1, ptr @g_arr, align 4
  store i32 2, ptr getelementptr (i32, ptr @g_arr, i64 1), align 4
  store i32 3, ptr getelementptr (i32, ptr @g_arr, i64 2), align 4
  %2 = load i32, ptr @g, align 4
  store i32 %2, ptr getelementptr (i32, ptr @g_arr, i64 3), align 4
  store ptr getelementptr (i32, ptr getelementptr (i32, ptr @g_arr, i64 3), i64 1), ptr %1, align 8
```
terapines-osc-cir pushed a commit to Terapines/clangir that referenced this pull request Sep 2, 2025
…vm#1280)

Resolves llvm#1266

After change:

```llvm
%1 = alloca ptr, i64 1, align 8
  store i32 1, ptr @g_arr, align 4
  store i32 2, ptr getelementptr (i32, ptr @g_arr, i64 1), align 4
  store i32 3, ptr getelementptr (i32, ptr @g_arr, i64 2), align 4
  %2 = load i32, ptr @g, align 4
  store i32 %2, ptr getelementptr (i32, ptr @g_arr, i64 3), align 4
  store ptr getelementptr (i32, ptr getelementptr (i32, ptr @g_arr, i64 3), i64 1), ptr %1, align 8
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Codegen of array initialization generates weird LLVM-IR

2 participants