Open Quuxplusone opened 13 years ago
Bugzilla Link | PR10256 |
Status | NEW |
Importance | P normal |
Reported by | Igor Galić (i.galic@brainsware.org) |
Reported on | 2011-07-03 15:37:26 -0700 |
Last modified on | 2011-10-10 09:59:07 -0700 |
Version | trunk |
Hardware | PC Linux |
CC | chandlerc@gmail.com, dgregor@apple.com, llvm-bugs@lists.llvm.org, nicolasweber@gmx.de, scshunt@csclub.uwaterloo.ca |
Fixed by commit(s) | |
Attachments | |
Blocks | |
Blocked by | |
See also |
The essential problem here is that we're trying to copy a volatile qualified
member using a routine that accepts a non-volatile qualified void*. Clang
complains about this and refuses to build the __builtin_memcpy() call. The
Clang asserts because we don't ever expect that call to be invalid.
My first question is: is this code well formed? I think it is from my reading
of the standard, but I'd like a second opinion.
If so, I think we'll need to add code that correctly copies volatile qualified
elements.
That leads to my second question: are there any requirements imposed on the
copy by the volatile qualifier? If so, what requirements? Should we just strip
it and hand it to __builtin_mempcy, should we add a __builtin_memcpy that
accepts a volatile qualified void*, or should we generate code to copy exactly
one element at a time instead of copying "the byte range" arbitrarily?
I don't think we can optimize to a memcpy on volatile code, since the semantics of the abstract machine are memberwise copy, and thus we at least have to have a discernible point at which some but not all copies have been performed.
Here's how different compilers treat different test cases. I have at my
disposal: g++ on linux/x86_64 and different versions of Sun CC on Solaris
x86(_64)/sparc(v9).
Test case 1:
struct A {
unsigned int i;
volatile int buff[2];
};
int test(A* a1, A* a2) {
*a1 = *a2;
return 0;
}
==========================================================================
==========================================================================
Result with GCC:
.file "test1.cc"
.text
.p2align 4,,15
.globl test(A*, A*)
.type test(A*, A*), @function
test(A*, A*):
.LFB0:
.cfi_startproc
movq (%rsi), %rax
movq %rax, (%rdi)
movl 8(%rsi), %eax
movl %eax, 8(%rdi)
xorl %eax, %eax
ret
.cfi_endproc
.LFE0:
.size test(A*, A*), .-test(A*, A*)
.ident "GCC: (Ubuntu/Linaro 4.5.2-8ubuntu4) 4.5.2"
.section .note.GNU-stack,"",@progbits
========================================================================
========================================================================
Result with Sun CC:
.file "test1.cc"
.code32
.globl int test(A*,A*)
.type int test(A*,A*), @function
.local Dlrodata.lrodata
.local Dldata.ldata
.ident "iropt: Sun Compiler Common 12.2 SunOS_i386 Patch 145357-03 2011/03/08"
.ident "ir2hf: Sun Compiler Common 12.2 SunOS_i386 Patch 145357-03 2011/03/08"
.ident "ube: Sun Compiler Common 12.2 SunOS_i386 Patch 145357-03 2011/03/08"
.section .text,"ax"
{ 6 } .align 4
/================================================================================
/ FUNCTION int test(A*,A*)
/ %rbp - used as frame pointer.
/ arg0 "a1": 4(%esp),
/ arg1 "a2": 8(%esp),
int test(A*,A*):
.CG2:
/ BLOCK: 3, pred: 1, succ: 2, count: 1.00000
/ FILE test1.cc
/ 1 !struct A {
/ 2 ! unsigned int i;
/ 3 ! volatile int buff[2];
/ 4 !};
/ 6 !int test(A* a1, A* a2) {
{ 6 } push %ebp
{ 6 } movl %esp,%ebp
{ 6 } push %ebx
/ 7 ! *a1 = *a2;
{ 7 } movl 8(%ebp),%ebx / sym=a1
{ 7 } movl 12(%ebp),%eax / sym=a2
{ 7 } movl (%eax),%ecx
{ 7 } movl 4(%eax),%edx
{ 7 } movl %ecx,(%ebx)
{ 7 } movl %edx,4(%ebx)
{ 7 } movl 8(%eax),%eax
{ 7 } movl %eax,8(%ebx)
/ 8 ! return 0;
{ 8 } xorl %eax,%eax
{ 8 } pop %ebx
{ 8 } leave
{ 8 } ret
.size int test(A*,A*), . - int test(A*,A*)
.CG3:
.CG4:
.section .data,"aw"
Ddata.data: / Offset 0
.section .bss,"aw"
Bbss.bss:
.section .bssf,"aw"
.section .rodata,"a"
Drodata.rodata: / Offset 0
.section .picdata,"aw"
Dpicdata.picdata: / Offset 0
.section .lbss,"awh"
.type Blbss.lbss, @object
Blbss.lbss:
.section .ldata,"awh"
Dldata.ldata: / Offset 0
.type Dldata.ldata, @object
.section .lrodata,"ah"
Dlrodata.lrodata: / Offset 0
.type Dlrodata.lrodata, @object
.section .annotate
/ ANNOTATION: header
.string "anotate"
.2byte 4
.2byte 0
.4byte .CG1 - .CG0
.CG0:
/ ANNOTATION: Module
.2byte 0
.2byte .CG6 - .CG5
.CG5:
.4byte .text
.4byte .CG4 - .text
.CG6:
/ ANNOTATION: Function
.2byte 1
.2byte .CG8 - .CG7
.CG7:
.4byte .CG2
.4byte .CG4 - .CG2
.4byte 262656
.CG8:
.CG1:
/ ANNOTATION: end
// test2.cc
struct A {
unsigned int i;
int buff[2];
};
int test(A* a1, A* a2) {
*a1 = *a2;
return 0;
}
==========================================================================
==========================================================================
Result with GCC:
.file "test2.cc"
.text
.p2align 4,,15
.globl test(A*, A*)
.type test(A*, A*), @function
test(A*, A*):
.LFB0:
.cfi_startproc
movq (%rsi), %rax
movq %rax, (%rdi)
movl 8(%rsi), %eax
movl %eax, 8(%rdi)
xorl %eax, %eax
ret
.cfi_endproc
.LFE0:
.size test(A*, A*), .-test(A*, A*)
.ident "GCC: (Ubuntu/Linaro 4.5.2-8ubuntu4) 4.5.2"
.section .note.GNU-stack,"",@progbits
==========================================================================
==========================================================================
Result with Sun CC
/ FUNCTION int test(A*,A*)
/ %rbp - used as frame pointer.
/ arg0 "a1": 4(%esp),
/ arg1 "a2": 8(%esp),
int test(A*,A*):
.CG2:
/ BLOCK: 3, pred: 1, succ: 2, count: 1.00000
/ FILE test2.cc
/ 1 !struct A {
/ 2 ! unsigned int i;
/ 3 ! int buff[2];
/ 4 !};
/ 6 !int test(A* a1, A* a2) {
{ 6 } push %ebp
{ 6 } movl %esp,%ebp
{ 6 } push %ebx
/ 7 ! *a1 = *a2;
{ 7 } movl 8(%ebp),%ebx / sym=a1
{ 7 } movl 12(%ebp),%eax / sym=a2
{ 7 } movl (%eax),%ecx
{ 7 } movl 4(%eax),%edx
{ 7 } movl %ecx,(%ebx)
{ 7 } movl %edx,4(%ebx)
{ 7 } movl 8(%eax),%eax
{ 7 } movl %eax,8(%ebx)
/ 8 ! return 0;
{ 8 } xorl %eax,%eax
{ 8 } pop %ebx
{ 8 } leave
{ 8 } ret
.size int test(A*,A*), . - int test(A*,A*)
(Not pasting data sections, as it looks the exact same)
// test3.cc
struct A {
unsigned int i;
int buff[2];
};
int test(volatile A* a1, volatile A* a2) {
*a1 = *a2;
return 0;
}
==========================================================================
==========================================================================
Result with GCC:
igalic@tynix ~/src/llvmbug % g++ -O3 -S test3.cc
test3.cc: In function ‘int test(volatile A*, volatile A*)’:
test3.cc:7:16: error: no match for ‘operator=’ in ‘* a1 = * a2’
test3.cc:1:10: note: candidate is: A& A::operator=(const A&)
1 igalic@tynix ~/src/llvmbug %
==========================================================================
==========================================================================
Result with Sun CC:
/ FUNCTION int test(volatile A*,volatile A*)
/ %rbp - used as frame pointer.
/ arg0 "a1": 4(%esp),
/ arg1 "a2": 8(%esp),
int test(volatile A*,volatile A*):
.CG2:
/ BLOCK: 3, pred: 1, succ: 2, count: 1.00000
/ FILE test3.cc
/ 1 !struct A {
/ 2 ! unsigned int i;
/ 3 ! int buff[2];
/ 4 !};
/ 6 !int test(volatile A* a1, volatile A* a2) {
{ 6 } push %ebp
{ 6 } movl %esp,%ebp
{ 6 } subl $24,%esp
/ 7 ! *a1 = *a2;
{ 7 } movl 12(%ebp),%edx / sym=a2
{ 7 } movl (%edx),%eax
{ 7 } movl 4(%edx),%ecx
{ 7 } movl %eax,-24(%ebp) / sym=.TP0
{ 7 } movl %ecx,-20(%ebp) / sym=.TP0
{ 7 } movl 8(%edx),%eax
{ 7 } movl %eax,-16(%ebp) / sym=.TP0
{ 7 } movl 8(%ebp),%eax / sym=a1
{ 7 } movl -24(%ebp),%ecx / sym=.TP0
{ 7 } movl -20(%ebp),%edx / sym=.TP0
{ 7 } movl %ecx,(%eax)
{ 7 } movl %edx,4(%eax)
{ 7 } movl -16(%ebp),%ecx / sym=.TP0
{ 7 } movl %ecx,8(%eax)
/ 8 ! return 0;
{ 8 } xorl %eax,%eax
{ 8 } leave
{ 8 } ret
.size int test(volatile A*,volatile A*), . - int test(volatile A*,volatile A*)
// test4.cc
struct A {
unsigned int i;
volatile int buff[2];
};
int test(volatile A* a1, volatile A* a2) {
*a1 = *a2;
return 0;
}
==========================================================================
==========================================================================
Result with GCC:
igalic@tynix ~/src/llvmbug % g++ -O3 test4.cc -S
test4.cc: In function ‘int test(volatile A*, volatile A*)’:
test4.cc:7:16: error: no match for ‘operator=’ in ‘* a1 = * a2’
test4.cc:1:10: note: candidate is: A& A::operator=(const A&)
1 igalic@tynix ~/src/llvmbug %
==========================================================================
==========================================================================
Result with Sun CC:
/ FUNCTION int test(volatile A*,volatile A*)
/ %rbp - used as frame pointer.
/ arg0 "a1": 4(%esp),
/ arg1 "a2": 8(%esp),
int test(volatile A*,volatile A*):
.CG2:
/ BLOCK: 3, pred: 1, succ: 2, count: 1.00000
/ FILE test4.cc
/ 1 !struct A {
/ 2 ! unsigned int i;
/ 3 ! volatile int buff[2];
/ 4 !};
/ 6 !int test(volatile A* a1, volatile A* a2) {
{ 6 } push %ebp
{ 6 } movl %esp,%ebp
{ 6 } subl $24,%esp
/ 7 ! *a1 = *a2;
{ 7 } movl 12(%ebp),%edx / sym=a2
{ 7 } movl (%edx),%eax
{ 7 } movl 4(%edx),%ecx
{ 7 } movl %eax,-24(%ebp) / sym=.TP0
{ 7 } movl %ecx,-20(%ebp) / sym=.TP0
{ 7 } movl 8(%edx),%eax
{ 7 } movl %eax,-16(%ebp) / sym=.TP0
{ 7 } movl 8(%ebp),%eax / sym=a1
{ 7 } movl -24(%ebp),%ecx / sym=.TP0
{ 7 } movl -20(%ebp),%edx / sym=.TP0
{ 7 } movl %ecx,(%eax)
{ 7 } movl %edx,4(%eax)
{ 7 } movl -16(%ebp),%ecx / sym=.TP0
{ 7 } movl %ecx,8(%eax)
/ 8 ! return 0;
{ 8 } xorl %eax,%eax
{ 8 } leave
{ 8 } ret
.size int test(volatile A*,volatile A*), . - int test(volatile A*,volatile A*)
// test5.cc
struct A {
unsigned int i;
volatile int buff[2];
};
void test(volatile A* a) {
a->i = 1;
a->buff[0] = 7;
a->buff[1] = 18;
}
==========================================================================
==========================================================================
Result with GCC:
.file "test5.cc"
.text
.p2align 4,,15
.globl test(A volatile*)
.type test(A volatile*), @function
test(A volatile*):
.LFB0:
.cfi_startproc
movl $1, (%rdi)
movl $7, 4(%rdi)
movl $18, 8(%rdi)
ret
.cfi_endproc
.LFE0:
.size test(A volatile*), .-test(A volatile*)
.ident "GCC: (Ubuntu/Linaro 4.5.2-8ubuntu4) 4.5.2"
.section .note.GNU-stack,"",@progbits
==========================================================================
==========================================================================
Result with Sun CC:
/ FUNCTION void test(volatile A*)
/ %rbp - used as frame pointer.
/ arg0 "a": 4(%esp),
void test(volatile A*):
.CG2:
/ BLOCK: 3, pred: 1, succ: 2, count: 1.00000
/ FILE test5.cc
/ 1 !struct A {
/ 2 ! unsigned int i;
/ 3 ! volatile int buff[2];
/ 4 !};
/ 6 !void test(volatile A* a) {
{ 6 } push %ebp
{ 6 } movl %esp,%ebp
/ 7 ! a->i = 1;
{ 7 } movl 8(%ebp),%eax / sym=a
{ 7 } movl $1,(%eax)
/ 8 ! a->buff[0] = 7;
{ 8 } movl $7,4(%eax)
/ 9 ! a->buff[1] = 18;
{ 9 } movl $18,8(%eax)
/ 10 !}
{ 10 } leave
{ 10 } ret
.size void test(volatile A*), . - void test(volatile A*)
These are very simple example cases, which show case a very simple copying. Volatile is not always ignored by the compiler:
We no longer assert() on this. However, the class is trivially copyable (per the C++ standard), so we generate a memcpy()... which seems unfortunate.