-
Notifications
You must be signed in to change notification settings - Fork 106
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.
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
[CIR][CIRGen] Simplify LLVM IR array initialization for clang CIR #1280
base: main
Are you sure you want to change the base?
Conversation
This change should be done for the zero-fill as well I believe in the do while part. |
Thanks for working on this! Next step is to change/add testcases for you change, if you run |
1dfbb50
to
02e18ab
Compare
Is there any automation tool for // 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};
} |
You might be able to get something using
Please do like the status quo:
|
56aa999
to
0aa5333
Compare
Done with the changes, let me know if anything else is needed. |
There was a problem hiding this 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.
Could you land this for me, the tests have passed? |
Resolves #1266
After change: