Skip to content
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

[CIR][Lowering] Fix Global Attr Lowering #906

Merged
merged 1 commit into from
Oct 2, 2024
Merged

[CIR][Lowering] Fix Global Attr Lowering #906

merged 1 commit into from
Oct 2, 2024

Conversation

bruteforceboy
Copy link
Contributor

Consider the following code snippet tmp.c:

#define N 3200

struct S {
  double a[N];
  double b[N];
} s;

double *b = s.b;

void foo() {
  double x = 0;
  for (int i = 0; i < N; i++)
    x += b[i];
}

int main() {
  foo();
  return 0;
}

Running bin/clang tmp.c -fclangir -o tmp && ./tmp causes a segmentation fault.

I compared the LLVM IR with and without CIR and noticed a difference which causes this:
@b = global ptr getelementptr inbounds (%struct.S, ptr @s, i32 0, i32 1) // no CIR
@b = global ptr getelementptr inbounds (%struct.S, ptr @s, i32 1) // with CIR

It seems there is a missing index when creating global pointers from structs. I have updated Lowering/DirectToLLVM/LowerToLLVM.cpp, and added a few tests.

@smeenai
Copy link
Collaborator

smeenai commented Oct 1, 2024

How come you closed this? Was it fixed some other way?

@bruteforceboy
Copy link
Contributor Author

How come you closed this? Was it fixed some other way?

There is a small error I noticed. I will make another PR in a few hours.

@smeenai
Copy link
Collaborator

smeenai commented Oct 1, 2024

Got it. There should be an option to convert the PR to a draft and then convert it back if you don't want to have to close the PR :) (You should also be able to reopen this one instead of creating a new PR if you want, either way is fine.)

@bruteforceboy bruteforceboy reopened this Oct 2, 2024
@bruteforceboy bruteforceboy reopened this Oct 2, 2024
@bruteforceboy
Copy link
Contributor Author

I have no idea why the macOS tests here fail. The Ubuntu tests seem fine. @bcardosolopes can you please have a look?

@smeenai
Copy link
Collaborator

smeenai commented Oct 2, 2024

It seems like it might be a weird infra error, I triggered a retry.

bcardosolopes
bcardosolopes approved these changes Oct 2, 2024
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.

Awesome, thanks! Once the macOS test pass I'll merge it

@bcardosolopes bcardosolopes merged commit 7051000 into llvm:main Oct 2, 2024
9 of 10 checks passed
smeenai pushed a commit to smeenai/clangir that referenced this pull request Oct 9, 2024
@bruteforceboy @smeenai
[CIR][Lowering] Fix Global Attr Lowering (llvm#906)
775b29c 
Consider the following code snippet `tmp.c`: 
```
#define N 3200

struct S {
  double a[N];
  double b[N];
} s;

double *b = s.b;

void foo() {
  double x = 0;
  for (int i = 0; i < N; i++)
    x += b[i];
}

int main() {
  foo();
  return 0;
}
```
Running `bin/clang tmp.c -fclangir -o tmp && ./tmp` causes a
segmentation fault.

I compared the LLVM IR with and without CIR and noticed a difference
which causes this:
`@b = global ptr getelementptr inbounds (%struct.S, ptr @s, i32 0, i32
1)` // no CIR
`@b = global ptr getelementptr inbounds (%struct.S, ptr @s, i32 1)` //
with CIR

It seems there is a missing index when creating global pointers from
structs. I have updated `Lowering/DirectToLLVM/LowerToLLVM.cpp`, and
added a few tests.
smeenai pushed a commit to smeenai/clangir that referenced this pull request Oct 9, 2024
@bruteforceboy @smeenai
[CIR][Lowering] Fix Global Attr Lowering (llvm#906)
7ab9126 
Consider the following code snippet `tmp.c`: 
```
#define N 3200

struct S {
  double a[N];
  double b[N];
} s;

double *b = s.b;

void foo() {
  double x = 0;
  for (int i = 0; i < N; i++)
    x += b[i];
}

int main() {
  foo();
  return 0;
}
```
Running `bin/clang tmp.c -fclangir -o tmp && ./tmp` causes a
segmentation fault.

I compared the LLVM IR with and without CIR and noticed a difference
which causes this:
`@b = global ptr getelementptr inbounds (%struct.S, ptr @s, i32 0, i32
1)` // no CIR
`@b = global ptr getelementptr inbounds (%struct.S, ptr @s, i32 1)` //
with CIR

It seems there is a missing index when creating global pointers from
structs. I have updated `Lowering/DirectToLLVM/LowerToLLVM.cpp`, and
added a few tests.
smeenai pushed a commit to smeenai/clangir that referenced this pull request Oct 9, 2024
@bruteforceboy @smeenai
[CIR][Lowering] Fix Global Attr Lowering (llvm#906)
2c9df4e 
Consider the following code snippet `tmp.c`: 
```
#define N 3200

struct S {
  double a[N];
  double b[N];
} s;

double *b = s.b;

void foo() {
  double x = 0;
  for (int i = 0; i < N; i++)
    x += b[i];
}

int main() {
  foo();
  return 0;
}
```
Running `bin/clang tmp.c -fclangir -o tmp && ./tmp` causes a
segmentation fault.

I compared the LLVM IR with and without CIR and noticed a difference
which causes this:
`@b = global ptr getelementptr inbounds (%struct.S, ptr @s, i32 0, i32
1)` // no CIR
`@b = global ptr getelementptr inbounds (%struct.S, ptr @s, i32 1)` //
with CIR

It seems there is a missing index when creating global pointers from
structs. I have updated `Lowering/DirectToLLVM/LowerToLLVM.cpp`, and
added a few tests.
keryell pushed a commit to keryell/clangir that referenced this pull request Oct 19, 2024
@bruteforceboy @lanza
[CIR][Lowering] Fix Global Attr Lowering (llvm#906)
3966c67 
Consider the following code snippet `tmp.c`: 
```
#define N 3200

struct S {
  double a[N];
  double b[N];
} s;

double *b = s.b;

void foo() {
  double x = 0;
  for (int i = 0; i < N; i++)
    x += b[i];
}

int main() {
  foo();
  return 0;
}
```
Running `bin/clang tmp.c -fclangir -o tmp && ./tmp` causes a
segmentation fault.

I compared the LLVM IR with and without CIR and noticed a difference
which causes this:
`@b = global ptr getelementptr inbounds (%struct.S, ptr @s, i32 0, i32
1)` // no CIR
`@b = global ptr getelementptr inbounds (%struct.S, ptr @s, i32 1)` //
with CIR

It seems there is a missing index when creating global pointers from
structs. I have updated `Lowering/DirectToLLVM/LowerToLLVM.cpp`, and
added a few tests.
lanza pushed a commit that referenced this pull request Nov 5, 2024
@bruteforceboy @lanza
[CIR][Lowering] Fix Global Attr Lowering (#906)
76b3dc5 
Consider the following code snippet `tmp.c`: 
```
#define N 3200

struct S {
  double a[N];
  double b[N];
} s;

double *b = s.b;

void foo() {
  double x = 0;
  for (int i = 0; i < N; i++)
    x += b[i];
}

int main() {
  foo();
  return 0;
}
```
Running `bin/clang tmp.c -fclangir -o tmp && ./tmp` causes a
segmentation fault.

I compared the LLVM IR with and without CIR and noticed a difference
which causes this:
`@b = global ptr getelementptr inbounds (%struct.S, ptr @s, i32 0, i32
1)` // no CIR
`@b = global ptr getelementptr inbounds (%struct.S, ptr @s, i32 1)` //
with CIR

It seems there is a missing index when creating global pointers from
structs. I have updated `Lowering/DirectToLLVM/LowerToLLVM.cpp`, and
added a few tests.
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

@lanza lanza Awaiting requested review from lanza lanza is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@bruteforceboy @smeenai @bcardosolopes