Re: [llvm-commits] [llvm-gcc-4.2] r43399 - /llvm-gcc-4.2/trunk/gcc/llvm-convert.cpp
Hi Dale, While that's nonstandard, it's hard to imagine a memcpy implementation that would screw it up. But they can partially overlap: how does mainline handle this? I know it used to get it wrong (the testcase I sent was filched from gcc bugzilla), but the bug was marked as resolved, without a link to the resolving changeset. Maybe they only solved the memcpy from a location to itself problem, and not the overlapping case? Ciao, D. ___ llvm-commits mailing list llvm-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
Re: [llvm-commits] [llvm-gcc-4.2] r43399 - /llvm-gcc-4.2/trunk/gcc/llvm-convert.cpp
Hi Bill, the following testcase shows another problem:
struct A
{
int a[1024];
};
void g(struct A *a, struct A *b)
{
*a = *b;
}
struct A c;
int main(void)
{
g(c, c);
}
Note that llvm-gcc generates a memcpy for the *a = *b
assignment, but it should be memmove since *a and *b
may be the same (as they are in this case).
Ciao,
Duncan.
PS: on some machines memcpy goes faster if you zero out
the destination before doing the copy. This shows why
doing a memcpy of p to p is a bad idea!
___
llvm-commits mailing list
llvm-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
Re: [llvm-commits] [llvm-gcc-4.2] r43399 - /llvm-gcc-4.2/trunk/gcc/llvm-convert.cpp
Hi Bill,
Does this help? :-)
the tree dumps weren't very informative! :) Here is a testcase that
seems to show the same kind of problem, and works on my machine:
struct A
{
int a[11];
};
void Qux() {
static struct A C = {0};
struct A __attribute__ ((aligned (1))) Bar;
Bar = C;
}
Can you please confirm that this is the problem you are talking about?
Thanks,
Duncan.
___
llvm-commits mailing list
llvm-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
Re: [llvm-commits] [llvm-gcc-4.2] r43399 - /llvm-gcc-4.2/trunk/gcc/llvm-convert.cpp
On Nov 7, 2007, at 1:20 AM, Duncan Sands wrote:
Hi Bill,
Does this help? :-)
the tree dumps weren't very informative! :)
Yeah...there weren't that many that were dumped. :-)
Here is a testcase that seems to show the same kind of problem, and
works on my machine:
struct A
{
int a[11];
};
void Qux() {
static struct A C = {0};
struct A __attribute__ ((aligned (1))) Bar;
Bar = C;
}
Can you please confirm that this is the problem you are talking about?
No, this doesn't give the assert. :-(
-bw
___
llvm-commits mailing list
llvm-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
Re: [llvm-commits] [llvm-gcc-4.2] r43399 - /llvm-gcc-4.2/trunk/gcc/llvm-convert.cpp
Hi Bill, Can you please confirm that this is the problem you are talking about? No, this doesn't give the assert. :-( the memcpy seems to have the wrong alignment in the LLVM IR. This may or may not lead to an assert later I suppose. I meant: is the memcpy getting the wrong alignment for the same reason as in your testcase. Thanks, Duncan. ___ llvm-commits mailing list llvm-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
Re: [llvm-commits] [llvm-gcc-4.2] r43399 - /llvm-gcc-4.2/trunk/gcc/llvm-convert.cpp
Hi Duncan, No, this doesn't give the assert. :-( the memcpy seems to have the wrong alignment in the LLVM IR. This may or may not lead to an assert later I suppose. That's what's wrong here. We're using the alignment from the source pointer as the alignment for the memcpy, which is wrong for the destination pointer. I meant: is the memcpy getting the wrong alignment for the same reason as in your testcase. I believe so. It should also be asserting, but isn't. There's another discussion of this on LLVM-Dev RFC: llvm-convert.cpp Patch. No one likes my original patch, but I just wrote another one that might be more palatable. :-) -bw ___ llvm-commits mailing list llvm-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
Re: [llvm-commits] [llvm-gcc-4.2] r43399 - /llvm-gcc-4.2/trunk/gcc/llvm-convert.cpp
On Nov 7, 2007, at 12:46 AM, Duncan Sands wrote:
Hi Bill, the following testcase shows another problem:
struct A
{
int a[1024];
};
void g(struct A *a, struct A *b)
{
*a = *b;
}
struct A c;
int main(void)
{
g(c, c);
}
Note that llvm-gcc generates a memcpy for the *a = *b
assignment, but it should be memmove since *a and *b
may be the same (as they are in this case).
While that's nonstandard, it's hard to imagine a memcpy
implementation that would screw it up.
But they can partially overlap:
struct A { int a[1024]; }
struct B { int x; struct A y; };
union C { struct A x; struct B y; }
void g(struct A* a, struct A* b) { *a = *b; }
union C u;
main(void) { g(u.x, u.y.y);
I fixed this for structure return values a while back (43324) but
you're right that the problem is more general.
___
llvm-commits mailing list
llvm-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
Re: [llvm-commits] [llvm-gcc-4.2] r43399 - /llvm-gcc-4.2/trunk/gcc/llvm-convert.cpp
On 10/26/07, Duncan Sands [EMAIL PROTECTED] wrote:
I've only seen this problem on PPC64. In particular, it's writing this
information into the redzone because this is a leaf function, and
thus the destination may be unaligned.
Can you please send me the gcc tree dumps, and also what debug_tree
gives for the modify_expr.
Here's the debug_tree:
Breakpoint 1, TreeToLLVM::EmitMODIFY_EXPR (this=0xbfffe998,
exp=0x40d11e70, DestLoc=0x0) at
/Volumes/SandBox/Clean/llvm--01.roots/llvm--01~obj/\
src/gcc/llvm-convert.cpp:3024
(gdb) p debug_tree(exp)
modify_expr 0x40d11e70
type void_type 0x40d0f880 void VOID
align 8 symtab 1 alias set -1
LLVM: void
pointer_to_this pointer_type 0x40d0f900
side-effects
arg 0 var_decl 0x40d7e180 Bar
type array_type 0x40d7e100 type integer_type 0x40d05300 char
sizes-gimplified BLK
size integer_cst 0x40d11cc0 constant invariant 88
unit size integer_cst 0x40d00f60 constant invariant 11
align 8 symtab 3 alias set -1 domain integer_type 0x40d7e080
LLVM: [11 x i8]
used asm-frame-size 0 BLK file testcase.c line 2 size
integer_cst 0x40d11cc0 88 unit size integer_cst 0x40d00f60 11
align 8 context function_decl 0x40d74e80 Qux
LLVM: [11 x i8]* %Bar
arg 1 var_decl 0x40d7e280 C.0 type array_type 0x40d7e100
readonly asm_written used static ignored asm-frame-size 0 BLK
file testcase.c line 2 size integer_cst 0x40d11cc0 88 unit size
integer_cst 0x\
40d00f60 11
align 64 context function_decl 0x40d74e80 Qux initial
constructor 0x40d11db0
LLVM: [11 x i8]* @C.0.1173 chain var_decl 0x40d7e180 Bar
testcase.c:2
$1 = void
The tree dumps:
$ cat testcase.c.t03.generic
Qux ()
{
static char C.0[11] = {0};
char Bar[11];
Bar = C.0;
}
$ cat testcase.c.t02.original
;; Function Qux (Qux)
;; enabled by -tree-original
{
char Bar[11] = {0};
char Bar[11] = {0};
}
Does this help? :-)
-bw
___
llvm-commits mailing list
llvm-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
[llvm-commits] [llvm-gcc-4.2] r43399 - /llvm-gcc-4.2/trunk/gcc/llvm-convert.cpp
Author: void
Date: Fri Oct 26 15:27:28 2007
New Revision: 43399
URL: http://llvm.org/viewvc/llvm-project?rev=43399view=rev
Log:
Port of r43394:
There are situations where the source and destination alignments differ. This
can cause havoc with memcpy. In this case, memcpy is using the alignment of the
source pointer and assuming that the destination pointer has the same
alignment. Fix this by copying the source pointer node and adjusting the
alignment to the minimum of the two alignments.
Modified:
llvm-gcc-4.2/trunk/gcc/llvm-convert.cpp
Modified: llvm-gcc-4.2/trunk/gcc/llvm-convert.cpp
URL:
http://llvm.org/viewvc/llvm-project/llvm-gcc-4.2/trunk/gcc/llvm-convert.cpp?rev=43399r1=43398r2=43399view=diff
==
--- llvm-gcc-4.2/trunk/gcc/llvm-convert.cpp (original)
+++ llvm-gcc-4.2/trunk/gcc/llvm-convert.cpp Fri Oct 26 15:27:28 2007
@@ -2541,7 +2541,25 @@
EmitAggregateCopy(DestLoc, LV.Ptr, TREE_TYPE(exp), isVolatile, false,
Alignment);
} else if (!isVolatile TREE_CODE(TREE_OPERAND(exp, 0))!=RESULT_DECL) {
- Emit(TREE_OPERAND(exp, 1), LV.Ptr);
+ // At this point, Alignment is the alignment of the destination
+ // pointer. It may not match the alignment of the source pointer. So, we
+ // need to make sure that it's has at least its alignment.
+ tree new_exp = copy_node(TREE_OPERAND(exp, 1));
+ unsigned NewAlignment = expr_align(new_exp) / 8;
+ Alignment = (Alignment NewAlignment) ? Alignment : NewAlignment;
+ TYPE_ALIGN(TREE_TYPE(new_exp)) = Alignment;
+
+ switch (TREE_CODE(new_exp)) {
+ case VAR_DECL:
+ case PARM_DECL:
+ case RESULT_DECL:
+ DECL_ALIGN (new_exp) = Alignment * 8;
+ break;
+ default:
+ break;
+ }
+
+ Emit(new_exp, LV.Ptr);
} else {
// Need to do a volatile store into TREE_OPERAND(exp, 1). To do this, we
// emit it into a temporary memory location, then do a volatile copy into
___
llvm-commits mailing list
llvm-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
Re: [llvm-commits] [llvm-gcc-4.2] r43399 - /llvm-gcc-4.2/trunk/gcc/llvm-convert.cpp
Hi Bill, I must say that I don't like this patch at all. Is there no better way? I started to work on something better myself, but unfortunately on my machine (x86-32) the testcase doesn't lead to a memcpy with llvm-gcc-4.2 at all, only a memset. Can you please suggest a testcase that shows the problem with x86-32 linux. Thanks, Duncan. PS: Don't the other two branches of the if statement need the same treatment? ___ llvm-commits mailing list llvm-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
Re: [llvm-commits] [llvm-gcc-4.2] r43399 - /llvm-gcc-4.2/trunk/gcc/llvm-convert.cpp
Hi Duncan, Hi Bill, I must say that I don't like this patch at all. Is there no better way? I'm open to suggestions. :-) What part do you object to? I reasoned that we needed to make the memcpy intrinsic have the correct alignment. It is getting its alignment information from the source pointer's alignment, but this could conflict with the destination's alignment. I started to work on something better myself, but unfortunately on my machine (x86-32) the testcase doesn't lead to a memcpy with llvm-gcc-4.2 at all, only a memset. Can you please suggest a testcase that shows the problem with x86-32 linux. I've only seen this problem on PPC64. In particular, it's writing this information into the redzone because this is a leaf function, and thus the destination may be unaligned. PS: Don't the other two branches of the if statement need the same treatment? Possibly...I'll see if I can come up with a testcase for them that fails. -bw ___ llvm-commits mailing list llvm-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
Re: [llvm-commits] [llvm-gcc-4.2] r43399 - /llvm-gcc-4.2/trunk/gcc/llvm-convert.cpp
Hi Bill, I must say that I don't like this patch at all. Is there no better way? I'm open to suggestions. :-) What part do you object to? All of it! But let's not go there :) I've only seen this problem on PPC64. In particular, it's writing this information into the redzone because this is a leaf function, and thus the destination may be unaligned. Can you please send me the gcc tree dumps, and also what debug_tree gives for the modify_expr. Thanks, Duncan. ___ llvm-commits mailing list llvm-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
Re: [llvm-commits] [llvm-gcc-4.2] r43399 - /llvm-gcc-4.2/trunk/gcc/llvm-convert.cpp
On 10/26/07, Duncan Sands [EMAIL PROTECTED] wrote: Hi Bill, I must say that I don't like this patch at all. Is there no better way? I'm open to suggestions. :-) What part do you object to? All of it! But let's not go there :) I've only seen this problem on PPC64. In particular, it's writing this information into the redzone because this is a leaf function, and thus the destination may be unaligned. Can you please send me the gcc tree dumps, and also what debug_tree gives for the modify_expr. I'm no longer able to reproduce it with TOT minus my patch. I reverted the patches to llvm-convert.cpp in 4.0 and 4.2. I'm going to add a test to make sure that this continues to work. -bw ___ llvm-commits mailing list llvm-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
