-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
union aggregate copies broken in some cases #1793
Comments
assigned to @lattner |
Duncan, Would it be possible for you to verify whether PR 1419 patch fixes this or not ? Thanks. |
I don't see how it can, since there is no padding in this case. |
The PR 1419 patch makes no difference. |
This is almost certainly a very long standing bug in CopyAggregate. There is this fixme in
-Chris |
The solution for this bug is to change the behavior of EmitAggregateCopy. Right now, it calls The predicate should check to see if it is small, AND that the llvm type has fields that overlap all of the -Chris |
How about to just call "EmitBuiltinMemCopy" when the size is small? And on EmitBuiltinMemCopy, if the copying size is small enough, just emit iteration of int assignments? In my knowledge, access time for char and int is same or speed of int is a little faster. Quho |
Here is another nice simple example: I guess I can take a crack at fixing this. -Chris |
This fixes the llvm-gcc part: Testcase here: test/CFrontend/2007-05-29-UnionCopy.c -scalarrepl still needs a fix. -Chris |
The bug in llvm-gcc is fixed here: The bug in scalarrepl is fixed here: Testcases here: -Chris |
Urg, reduced testcases work, but the full ones don't. |
Here's the final fix: -Chris |
Extended Description
Testcase:
struct A {
char c;
int i;
};
struct B {
char c;
unsigned char x;
};
union U { struct A a; struct B b; };
int check (void) {
union U u, v;
v.b.x = 0xff;
u = v;
return (u.b.x == 0xff);
}
Compiled with -O2:
ret i32 0
Should return 1. The point is that the union takes its fields
from struct A. The second char x in struct B is in the empty
space (due to alignment) in struct A between char c and int i,
so is not copied when CopyAggregate copies by fields. Copying
by fields is completely bogus with respect to unions of structs
unless every field in every union member is covered by an LLVM
field. This cannot be done by converting only one union member,
as is done right now.
The text was updated successfully, but these errors were encountered: