Re: can't find VCG viewer
Hello Joe. On 3/14/07, Joe Buck <[EMAIL PROTECTED]> wrote: On Wed, Mar 14, 2007 at 11:36:24AM +0200, Sunzir Deepur wrote: > any idea where I can find a (free) graphical VCG viewer suitable > for gcc's vcg outputs ? See http://www.graphviz.org/ Checked on graphviz, I don't think it supports VCG files. I did find some scripts that converts dot files to vcgs, but not the opposite. Do you use graphviz to display VCG files ? maybe I missed something... thank you ! sunzir
Re: can't find VCG viewer
Hello Diego. On 3/15/07, Diego Novillo <[EMAIL PROTECTED]> wrote: Sunzir Deepur wrote on 03/14/07 05:36: > any idea where I can find a (free) graphical VCG viewer suitable > for gcc's vcg outputs ? I'd recommend the attached script. Feed the output to GraphViz. The script may need changes if you are using RTL dumps. Thank you for that. Unfortunately I'm interested in the rtl dumps (fdump-rtl-all)... Do you have any idea how can I view those vcg files ? (that 1995 vcg package does segmentation fault on GCC's vcg files) Thank you ! sunzir
VCG viewer...
Hi Guys, Sorry to bother you with that old issue... But.. Is there a chance someone has a secret stash with a working version of the VCG graph viewer ? please ? thank you ppl sunzir Re: Looking for Cygnus' modified VCG graph viewer To: drepper at cygnus dot com (Ulrich Drepper) Subject: Re: Looking for Cygnus' modified VCG graph viewer From: Jeffrey A Law Date: Sun, 17 Dec 2000 15:53:28 -0700 cc: Jamie Lokier , gcc at gcc dot gnu dot org Reply-To: law at redhat dot com In message <[EMAIL PROTECTED]>you write: > Jamie Lokier <[EMAIL PROTECTED]> writes: > > > which no longer exists. Can anyone tell me where to find the fixed VCG, > > or if it the fixes were found to be unnecessary after all? > > I've put the file back in place in the hope that whoever removed it > does not do it again. Remove it from gcc.gnu.org. We can't distribute it on that site due to licensing issues. jeff
Re: Integer overflow in operator new
* Joe Buck: > Consider an implementation that, when given > >Foo* array_of_foo = new Foo[n_elements]; > > passes __compute_size(elements, sizeof Foo) instead of n_elements*sizeof Foo > to operator new, where __compute_size is > > inline size_t __compute_size(size_t num, size_t size) { > size_t product = num * size; > return product >= num ? product : ~size_t(0); > } I don't think this check is correct. Consider num = 0x3334 and size = 6. It seems that the check is difficult to perform efficiently unless the architecture provides unsigned multiplication with overflow detection, or an instruction to implement __builtin_clz.
Re: x86 inc/dec on core2
H. J. Lu wrote: inc/dec has the same performance as add/sub on Core 2 Duo. But inc/dec is shorter. What about partial flag register dependency of inc/dec? There is no partial flag register dependency on inc/dec. My docs say that "INC/DEC does not change the carry flag". But you have better resources that I, so if you think that C2D should be left out of X86_TUNE_USE_INCDEC, then the patch is pre-approved for mainline. Thanks, Uros.
Re: x86 inc/dec on core2
Uros Bizjak wrote: My docs say that "INC/DEC does not change the carry flag". But you have better resources that I, so if you think that C2D should be left out of X86_TUNE_USE_INCDEC, then the patch is pre-approved for mainline. Absolutely INC/DEC do not change the carry flag, this is an important part of the architecture, how else would you code ADC loops?
RE: Integer overflow in operator new
On 08 April 2007 10:43, Florian Weimer wrote: > * Joe Buck: > >> Consider an implementation that, when given >> >> Foo* array_of_foo = new Foo[n_elements]; >> >> passes __compute_size(elements, sizeof Foo) instead of n_elements*sizeof >> Foo to operator new, where __compute_size is >> >> inline size_t __compute_size(size_t num, size_t size) { >> size_t product = num * size; >> return product >= num ? product : ~size_t(0); >> } > > I don't think this check is correct. Consider num = 0x3334 and > size = 6. It seems that the check is difficult to perform efficiently > unless the architecture provides unsigned multiplication with overflow > detection, or an instruction to implement __builtin_clz. Wouldn't using -ftrapv do what we want? Would a possible answer be to make an ftrapv attribute that could be selectively applied to security-critical library routines such as operator new? cheers, DaveK -- Can't think of a witty .sigline today
Re: Integer overflow in operator new
"Dave Korn" <[EMAIL PROTECTED]> writes: > Wouldn't using -ftrapv do what we want? -ftrapv only modifies signed arithmetic. Unsigned arithmetic never traps. Andreas. -- Andreas Schwab, SuSE Labs, [EMAIL PROTECTED] SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany PGP key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different."
RE: Integer overflow in operator new
On 08 April 2007 13:58, Andreas Schwab wrote: > "Dave Korn" <[EMAIL PROTECTED]> writes: > >> Wouldn't using -ftrapv do what we want? > > -ftrapv only modifies signed arithmetic. Unsigned arithmetic never traps. > > Andreas. Meh, I'm an idiot. Still, maybe we want to create a -futrapv flag/attribute? cheers, DaveK -- Can't think of a witty .sigline today
Re: x86 inc/dec on core2
On Sun, Apr 08, 2007 at 11:37:43AM +0200, Uros Bizjak wrote: > H. J. Lu wrote: > > >>>inc/dec has the same performance as add/sub on Core 2 Duo. But > >>>inc/dec is shorter. > >>> > >>> > >>What about partial flag register dependency of inc/dec? > >> > > > >There is no partial flag register dependency on inc/dec. > > > > My docs say that "INC/DEC does not change the carry flag". But you have > better resources that I, so if you think that C2D should be left out of > X86_TUNE_USE_INCDEC, then the patch is pre-approved for mainline. Partial flag register stall only applies to instructions like shift since they may shift by 0 in which case they won't change flag register. H.J.
Re: Integer overflow in operator new
In article <[EMAIL PROTECTED]> you write: >On Fri, Apr 06, 2007 at 06:51:24PM -0500, Gabriel Dos Reis wrote: >> David Daney <[EMAIL PROTECTED]> writes: >> >> | One could argue that issuing some type of diagnostic (either at >> | compile time or run time) would be helpful for people that don't >> | remember to write correct code 100% of the time. >> >> I raised this very issue a long time ago; a long-term GCC contributor >> vocally opposed checking the possible overflow. I hope something will >> happen this time. > >I don't like slowing programs down, but I like security holes even less. > >If a check were to be implemented, the right thing to do would be to throw >bad_alloc (for the default new) or return 0 (for the nothrow new). If a >process's virtual memory limit is 400M, we throw bad_alloc if we do new >int[200], so we might as well do it if we do new int[20]. >There would be no reason to use a different reporting mechanism. > >There might be rare cases where the penalty for this check could have >an impact, like for pool allocators that are otherwise very cheap. >If so, there could be a flag to suppress the check. > Considering the issue is only for new [], I'd assume anyone who wants less correct, but faster behavior would simply handle the computation themselves, and deal with the overflow manually, then override whatever operator/class they need to make things work.
Re: Integer overflow in operator new
In article <[EMAIL PROTECTED]> you write: >The assert should not overflow. I suggest > >#include >#include >assert( n < SIZE_MAX / sizeof(int) ); > >which requires two pieces of information that the programmer >otherwise wouldn't need, SIZE_MAX and sizeof(type). > >Asking programmers to write extra code for rare events, has >not been very successful. It would be better if the compiler >incorporated this check into operator new, though throwing >an exception rather than asserting. The compiler should be >able to eliminate many of the conditionals. The compiler and its runtime should be correct, and programmers should be able to depend on it. When you read the documentation for new or calloc, there is no mention of integer overflow, it is not expected that the programmer has to know about that. Adding an extra test in user code even less sense than checking that pointers are not null before calling free...
Re: Integer overflow in operator new
Dave Korn wrote: Wouldn't using -ftrapv do what we want? Would a possible answer be to make an ftrapv attribute that could be selectively applied to security-critical library routines such as operator new? I have always been told that -ftrapv is nowhere near fully working or reliable (I think Eric is the source of that advice). Right now we generate somewhat nasty code for required overflow checking in GNAT based on that advice.
Re: x86 inc/dec on core2
On Apr 8, 2007, at 2:37 AM, Uros Bizjak wrote: My docs say that "INC/DEC does not change the carry flag". Personally, I'm having a hard time envisioning how the semantics of the instruction are relevant at all. This is all about instructing tuning, so, semantics cannot matter, otherwise, it would be wrong to make this a tune choice. But you have better resources that I, so if you think that C2D should be left out of X86_TUNE_USE_INCDEC, then the patch is pre- approved for mainline. I'm confused again, it isn't that it should be left out, it is that it should be included. My patch adds inc/dec selection for C2D. I'd also like it for generic on darwin, as that makes more sense for us. How does the rest of the community feel about inc/dec selection for generic?
Re: x86 inc/dec on core2
Mike Stump wrote: On Apr 8, 2007, at 2:37 AM, Uros Bizjak wrote: My docs say that "INC/DEC does not change the carry flag". Personally, I'm having a hard time envisioning how the semantics of the instruction are relevant at all. This is all about instructing tuning, so, semantics cannot matter, otherwise, it would be wrong to make this a tune choice. Well for sure INC and ADD have different semantics. INC does not affect the carry flag, and ADD does, so there are definitely situations where you want one and the other won't work!
Re: Integer overflow in operator new
Joe Buck writes: > inline size_t __compute_size(size_t num, size_t size) { > size_t product = num * size; > return product >= num ? product : ~size_t(0); > } Florian Weimer writes: >I don't think this check is correct. Consider num = 0x3334 and >size = 6. It seems that the check is difficult to perform efficiently >unless the architecture provides unsigned multiplication with overflow >detection, or an instruction to implement __builtin_clz. This should work instead: inline size_t __compute_size(size_t num, size_t size) { if (num > ~size_t(0) / size) return ~size_t(0); return num * size; } Ross Ridge
Re: x86 inc/dec on core2
Mike Stump wrote: But you have better resources that I, so if you think that C2D should be left out of X86_TUNE_USE_INCDEC, then the patch is pre-approved for mainline. I'm confused again, it isn't that it should be left out, it is that it should be included. My patch adds inc/dec selection for C2D. I'd also like it for generic on darwin, as that makes more sense for us. How does the rest of the community feel about inc/dec selection for generic? Just to clear the mess - Yes, C2D should be a part of X86_TUNE_USE_INCDEC. Sorry for the confusion. Uros.
Re: Integer overflow in operator new
Robert Dewar wrote: I have always been told that -ftrapv is nowhere near fully working or reliable (I think Eric is the source of that advice). Is this just a rumor, or are there data that backs this up. (That - fwrapv doesn't work, not that Dewar was always told that it doesn't work.) Brad
Re: Integer overflow in operator new
On 4/8/07, Bradley Lucier <[EMAIL PROTECTED]> wrote: Is this just a rumor, or are there data that backs this up. (That - fwrapv doesn't work, not that Dewar was always told that it doesn't work.) If you look into the bugzilla, you will see now two bugs filed against -ftrapv. One because the rtl back-end removes the libcall and another because we optimize away the overflow checks in libgcc2.c (PR 19020 and PR 31477). I don't know why people can't look this up themselves, it is not like it is private information or anything. -- Pinski
Re: x86 inc/dec on core2
Mike Stump wrote: I was wondering, if: /* X86_TUNE_USE_INCDEC */ ~(m_PENT4 | m_NOCONA | m_CORE2 | m_GENERIC), is correct. Should it be: /* X86_TUNE_USE_INCDEC */ ~(m_PENT4 | m_NOCONA | m_GENERIC), ? In the original patch in: 2006-11-18 Vladimir Makarov <[EMAIL PROTECTED]> * doc/invoke.texi (core2): Add item. it wasn't present, but in the checked in patch in r118973, it is: $ svn diff -r118972:118973 i386.c | grep incdec -const int x86_use_incdec = ~(m_PENT4 | m_NOCONA | m_GENERIC); +const int x86_use_incdec = ~(m_PENT4 | m_NOCONA | m_CORE2 | m_GENERIC); It is probably a typo. I did several changes after original patch submission because people asked me to benhmark several parameters. Core2 really should generate INC/DEC because it results in smaller programs (as I remember about 0.4% and 0.1% correspondingly for SPECINT2000 and SPECFP2000) and I definitely remeber a bit better code too. I looked around for a discussion on this, and didn't find it. Thanks?
Re: Integer overflow in operator new
Andrew Pinski writes: > On 4/8/07, Bradley Lucier <[EMAIL PROTECTED]> wrote: > > Is this just a rumor, or are there data that backs this up. (That - > > fwrapv doesn't work, not that Dewar was always told that it doesn't > > work.) > > If you look into the bugzilla, you will see now two bugs filed against > -ftrapv. One because the rtl back-end removes the libcall and another > because we optimize away the overflow checks in libgcc2.c (PR 19020 > and PR 31477). I don't know why people can't look this up themselves, > it is not like it is private information or anything. You're really good at Bugzilla searches -- I find it really hard to locate anything in Bugzilla, but I've seen you can do so in an instant. So, I know it's possible, but... :-) Andrew. -- Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, UK Registered in England and Wales No. 3798903
Re: Integer overflow in operator new
* Ross Ridge: > Florian Weimer writes: >>I don't think this check is correct. Consider num = 0x3334 and >>size = 6. It seems that the check is difficult to perform efficiently >>unless the architecture provides unsigned multiplication with overflow >>detection, or an instruction to implement __builtin_clz. > > This should work instead: > > inline size_t __compute_size(size_t num, size_t size) { > if (num > ~size_t(0) / size) > return ~size_t(0); > return num * size; > } Yeah, but that division is fairly expensive if it can't be performed at compile time. OTOH, if __compute_size is inlined in all places, code size does increase somewhat.
Re: Integer overflow in operator new
Bradley Lucier wrote: Robert Dewar wrote: I have always been told that -ftrapv is nowhere near fully working or reliable (I think Eric is the source of that advice). Is this just a rumor, or are there data that backs this up. (That - fwrapv doesn't work, not that Dewar was always told that it doesn't work.) Well there are a couple of bugs filed, but the real issue is whether there really exists experience of routinely building large scale applications with this switch, and I am under the impression the answer is no, but of course I could be wrong. For Ada, the desire would be to use this as the default on all targets for all applications, which represents pretty strenuous usage!
Re: Integer overflow in operator new
On Monday, 9 April 2007 10:23, Florian Weimer wrote: > * Ross Ridge: > > Florian Weimer writes: > >>I don't think this check is correct. Consider num = 0x3334 and > >>size = 6. It seems that the check is difficult to perform > >> efficiently unless the architecture provides unsigned > >> multiplication with overflow detection, or an instruction to > >> implement __builtin_clz. > > > > This should work instead: > > > > inline size_t __compute_size(size_t num, size_t size) { > > if (num > ~size_t(0) / size) > > return ~size_t(0); > > return num * size; > > } > > Yeah, but that division is fairly expensive if it can't be performed > at compile time. OTOH, if __compute_size is inlined in all places, > code size does increase somewhat. You could avoid the division in nearly all cases by checking for reasonably-sized arguments first: inline size_t __compute_size(size_t num, size_t size) { static const int max_bits = sizeof(size_t) * CHAR_BITS; int low_num, low_size; low_num = num < ((size_t)1 << (max_bits * 5 / 8)); low_size = size < ((size_t)1 << (max_bits * 3 / 8)); if (__builtin_expect(low_num && low_size, 1) || num <= ~(size_t)0 / size) return num * size; else return ~size_t(0); } -- Ross Smith [EMAIL PROTECTED] Auckland, New Zealand "Those who can make you believe absurdities can make you commit atrocities."-- Voltaire
Re: Integer overflow in operator new
Joe Buck wrote: > > > inline size_t __compute_size(size_t num, size_t size) { > > > size_t product = num * size; > > > return product >= num ? product : ~size_t(0); > > > } 2007/4/9, Ross Smith <[EMAIL PROTECTED]> wrote: On Monday, 9 April 2007 10:23, Florian Weimer wrote: > * Ross Ridge: > > Florian Weimer writes: > >>I don't think this check is correct. Consider num = 0x3334 and > >>size = 6. It seems that the check is difficult to perform > >> efficiently unless the architecture provides unsigned > >> multiplication with overflow detection, or an instruction to > >> implement __builtin_clz. > > > > This should work instead: > > > > inline size_t __compute_size(size_t num, size_t size) { > > if (num > ~size_t(0) / size) > > return ~size_t(0); > > return num * size; > > } > > Yeah, but that division is fairly expensive if it can't be performed > at compile time. OTOH, if __compute_size is inlined in all places, > code size does increase somewhat. You could avoid the division in nearly all cases by checking for reasonably-sized arguments first: inline size_t __compute_size(size_t num, size_t size) { static const int max_bits = sizeof(size_t) * CHAR_BITS; int low_num, low_size; low_num = num < ((size_t)1 << (max_bits * 5 / 8)); low_size = size < ((size_t)1 << (max_bits * 3 / 8)); if (__builtin_expect(low_num && low_size, 1) || num <= ~(size_t)0 / size) return num * size; else return ~size_t(0); } This code is bigger than Joe Buck's. - Joe Buck's code: 10 instructions Ross Ridge's code: 16 instructions Ross Smith's code: 16 instructions - Joe Buck's code: 10 instructions __compute_size: pushl %ebp movl%esp, %ebp movl8(%ebp), %eax movl%eax, %edx imull 12(%ebp), %edx cmpl%eax, %edx orl $-1, %edx popl%ebp movl%edx, %eax ret Ross Ridge's code: 16 instructions __compute_size: pushl %ebp orl $-1, %eax movl%esp, %ebp xorl%edx, %edx movl12(%ebp), %ecx pushl %ebx movl8(%ebp), %ebx divl%ecx orl $-1, %edx cmpl%eax, %ebx movl%ebx, %edx imull %ecx, %edx popl%ebx movl%edx, %eax popl%ebp ret Ross Smith's code: 16 instructions __compute_size: pushl %ebp orl $-1, %eax movl%esp, %ebp xorl%edx, %edx movl12(%ebp), %ecx pushl %ebx movl8(%ebp), %ebx divl%ecx orl $-1, %edx cmpl%eax, %ebx movl%ebx, %edx imull %ecx, %edx popl%ebx movl%edx, %eax popl%ebp ret compute_size_april2007.tar.gz Description: GNU Zip compressed data
Re: Integer overflow in operator new
Joe Buck wrote: > > > inline size_t __compute_size(size_t num, size_t size) { > > > size_t product = num * size; > > > return product >= num ? product : ~size_t(0); > > > } 2007/4/9, Ross Smith <[EMAIL PROTECTED]> wrote: On Monday, 9 April 2007 10:23, Florian Weimer wrote: > * Ross Ridge: > > Florian Weimer writes: > >>I don't think this check is correct. Consider num = 0x3334 and > >>size = 6. It seems that the check is difficult to perform > >> efficiently unless the architecture provides unsigned > >> multiplication with overflow detection, or an instruction to > >> implement __builtin_clz. > > > > This should work instead: > > > > inline size_t __compute_size(size_t num, size_t size) { > > if (num > ~size_t(0) / size) > > return ~size_t(0); > > return num * size; > > } > > Yeah, but that division is fairly expensive if it can't be performed > at compile time. OTOH, if __compute_size is inlined in all places, > code size does increase somewhat. You could avoid the division in nearly all cases by checking for reasonably-sized arguments first: inline size_t __compute_size(size_t num, size_t size) { static const int max_bits = sizeof(size_t) * CHAR_BITS; int low_num, low_size; low_num = num < ((size_t)1 << (max_bits * 5 / 8)); low_size = size < ((size_t)1 << (max_bits * 3 / 8)); if (__builtin_expect(low_num && low_size, 1) || num <= ~(size_t)0 / size) return num * size; else return ~size_t(0); } This code is bigger than Joe Buck's. I'm sorry, the previous 3rd source code .c is an error mine. - Joe Buck's code: 10 instructions Ross Ridge's code: 16 instructions Ross Smith's code: 23 instructions - Joe Buck's code: 9 instructions __compute_size: pushl %ebp movl%esp, %ebp movl8(%ebp), %edx movl%edx, %eax imull 12(%ebp), %eax cmpl%edx, %eax orl $-1, %eax popl%ebp ret Ross Ridge's code: 16 instructions __compute_size: pushl %ebp movl%esp, %ebp orl $-1, %eax xorl%edx, %edx movl12(%ebp), %ecx divl%ecx pushl %ebx movl8(%ebp), %ebx orl $-1, %edx cmpl%eax, %ebx movl%ebx, %edx imull %ecx, %edx popl%ebx movl%edx, %eax popl%ebp ret Ross Smith's code: 23 instructions __compute_size: pushl %ebp movl%esp, %ebp pushl %ebx movl8(%ebp), %ebx cmpl$1048575, %ebx movl12(%ebp), %ecx setbe %dl xorl%eax, %eax cmpl$4095, %ecx setbe %al andb$1, %dl testl %eax, %eax orl $-1, %eax xorl%edx, %edx divl%ecx orl $-1, %edx cmpl%eax, %ebx movl%ebx, %edx imull %ecx, %edx popl%ebx movl%edx, %eax popl%ebp ret J.C. Pizarro
Re: Integer overflow in operator new
And this tarball. J.C. Pizarro. compute_size_april2007_2.tar.gz Description: GNU Zip compressed data
Re: Integer overflow in operator new
One instruction more in GCC-4.1.x vs GCC-3.4.6? Joe Buck's code: 10 instructions [ -Os of gcc-4.1.3-20070326 ] __compute_size: pushl %ebp movl%esp, %ebp movl8(%ebp), %eax movl%eax, %edx imull 12(%ebp), %edx cmpl%eax, %edx orl $-1, %edx popl%ebp movl%edx, %eax # <--- this extra instruction because return EAX = EDX? ret Joe Buck's code: 9 instructions [ -Os of gcc-3.4.6 ] __compute_size: pushl %ebp movl%esp, %ebp movl8(%ebp), %edx movl%edx, %eax imull 12(%ebp), %eax cmpl%edx, %eax orl $-1, %eax popl%ebp # <--- no extra instruction because return EAX = EAX? ret J.C. Pizarro
Miscompilation...
Hi all, This program: #include struct tree_type { unsigned int precision : 9; }; void *bork(const void *Ty, unsigned Subpart) { printf("Subpart == %08x\n", Subpart); return 0; } const void *TConvertType(tree_type* type) { asm("movl $1104150528, (%%esp)" : : ); const void *Ty = 0; return bork(Ty, type->precision); } const void *foo(tree_type* type) { asm("movl $1104150528, (%%esp)" : : ); const void *Ty = 0; unsigned S = type->precision; return bork(Ty, S); } int main() { struct tree_type t; t.precision = 1; TConvertType(&t); foo(&t); return 0; } Compiled with "c++ t.c" Should print out: Subpart == 0001 Subpart == 0001 But instead prints out: Subpart == 8fe50001 Subpart == 0001 (on my iMac). The problem seems to be that, in the TConvertType function passes "type->precision" as an HI instead of SI to the "bork" function. The asm code is: movl $1104150528, (%esp) movl$0, -4(%ebp) movl8(%ebp), %eax movzwl (%eax), %eax andw$511, %ax movw%ax, 4(%esp) movl-4(%ebp), %eax movl%eax, (%esp) call__Z4borkPKvj for TConvertType, so the %ax isn't putting a full SI onto the stack, leaving garbage in the MSBs of 4(%esp), which "bork" expects to be 0, of course. I believe I got the TOT -- .svn/entries says "svn://gcc.gnu.org/svn/ gcc/trunk". Is this a known problem? Thanks! -bw
Re: x86 inc/dec on core2
"Mike Stump" <[EMAIL PROTECTED]> ??:[EMAIL PROTECTED] > On Apr 8, 2007, at 2:37 AM, Uros Bizjak wrote: >> My docs say that "INC/DEC does not change the carry flag". > > Personally, I'm having a hard time envisioning how the semantics of the > instruction are relevant at all. This is all about instructing tuning, > so, semantics cannot matter, otherwise, it would be wrong to make this a > tune choice. Intel's optimization reference manual says that: 3.5.1.1 Use of the INC and DEC Instructions The INC and DEC instructions modify only a subset of the bits in the flag register. This creates a dependence on all previous writes of the flag register. This is especially problematic when these instructions are on the critical path because they are used to change an address for a load on which many other instructions depend. Assembly/Compiler Coding Rule 32. (M impact, H generality) INC and DEC instructions should be replaced with ADD or SUB instructions, because ADD and SUB overwrite all flags, whereas INC and DEC do not, therefore creating false dependencies on earlier instructions that set the flags. > >> But you have better resources that I, so if you think that C2D should be >> left out of X86_TUNE_USE_INCDEC, then the patch is pre- approved for >> mainline. > > I'm confused again, it isn't that it should be left out, it is that it > should be included. My patch adds inc/dec selection for C2D. I'd also > like it for generic on darwin, as that makes more sense for us. How does > the rest of the community feel about inc/dec selection for generic? -- Zuxy
Re: Miscompilation...
On Apr 8, 2007, at 6:50 PM, Bill Wendling wrote: Hi all, This program: #include struct tree_type { unsigned int precision : 9; }; void *bork(const void *Ty, unsigned Subpart) { printf("Subpart == %08x\n", Subpart); return 0; } const void *TConvertType(tree_type* type) { asm("movl $1104150528, (%%esp)" : : ); This should be asm("movl $1104150528, 4(%%esp)" : : ); const void *Ty = 0; return bork(Ty, type->precision); } const void *foo(tree_type* type) { asm("movl $1104150528, (%%esp)" : : ); Same here. const void *Ty = 0; unsigned S = type->precision; return bork(Ty, S); } int main() { struct tree_type t; t.precision = 1; TConvertType(&t); foo(&t); return 0; } Compiled with "c++ t.c" Should print out: Subpart == 0001 Subpart == 0001 But instead prints out: Subpart == 8fe50001 Subpart == 0001 Which means that this outputs: Subpart == 41d1 Subpart == 0001 (on my iMac). The problem seems to be that, in the TConvertType function passes "type->precision" as an HI instead of SI to the "bork" function. The asm code is: movl $1104150528, (%esp) And this would be: movl $1104150528, 4(%esp) movl$0, -4(%ebp) movl8(%ebp), %eax movzwl (%eax), %eax andw$511, %ax movw%ax, 4(%esp) movl-4(%ebp), %eax movl%eax, (%esp) call__Z4borkPKvj for TConvertType, so the %ax isn't putting a full SI onto the stack, leaving garbage in the MSBs of 4(%esp), which "bork" expects to be 0, of course. I believe I got the TOT -- .svn/entries says "svn://gcc.gnu.org/svn/ gcc/trunk". Is this a known problem? Thanks! -bw
Re: Integer overflow in operator new
> > Florian Weimer writes: > >>I don't think this check is correct. Consider num = 0x3334 and > >>size = 6. It seems that the check is difficult to perform efficiently > >>unless the architecture provides unsigned multiplication with overflow > >>detection, or an instruction to implement __builtin_clz. Right; sorry for the bad code. We need a saturating multiply, and the most efficient implementations can't be expressed in C/C++ directly. I don't think that fixing my code is the right approach. If there's an unsigned multiply instruction that sets an overflow flag, or a 32x32->64 unsigned multiply, then it suffices to say "if overflow, replace product with all-ones." The penalty for doing that should be a lot smaller.
RE: Miscompilation...
On 09 April 2007 02:51, Bill Wendling wrote: > (on my iMac). The problem seems to be that, in the TConvertType > function passes > "type->precision" as an HI instead of SI to the "bork" function. The > asm code is: > > movl $1104150528, (%esp) > movl$0, -4(%ebp) > movl8(%ebp), %eax > movzwl (%eax), %eax > andw$511, %ax > movw%ax, 4(%esp) > movl-4(%ebp), %eax > movl%eax, (%esp) > call__Z4borkPKvj > > for TConvertType, so the %ax isn't putting a full SI onto the stack, > leaving > garbage in the MSBs of 4(%esp), which "bork" expects to be 0, of course. Confirmed on x86/cygwin. I removed the asms just in case they were somehow confusing the compiler but the generated code is still clearly incorrect. 004010a2 <__Z12TConvertTypeP9tree_type>: 4010a2: 55 push %ebp 4010a3: 89 e5 mov%esp,%ebp 4010a5: 83 ec 18sub$0x18,%esp 4010a8: c7 45 fc 00 00 00 00movl $0x0,0xfffc(%ebp) 4010af: 8b 45 08mov0x8(%ebp),%eax 4010b2: 0f b7 00movzwl (%eax),%eax 4010b5: 66 25 ff 01 and$0x1ff,%ax 4010b9: 66 89 44 24 04 mov%ax,0x4(%esp) 4010be: 8b 45 fcmov0xfffc(%ebp),%eax 4010c1: 89 04 24mov%eax,(%esp) 4010c4: e8 87 ff ff ff call 401050 <__Z4borkPKvj> 4010c9: c9 leave 4010ca: c3 ret 4010cb: 90 nop Looking at tree dumps for the two functions ConvertType and foo, ;; Function const void* foo(tree_type*) (_Z3fooP9tree_type) const void* foo(tree_type*) (type) { unsigned int S; const void * Ty; void * D.2934; const void * D.2933; D.2932; # BLOCK 2 # PRED: ENTRY (fallthru) Ty = 0B; D.2932 = type->precision; S = (unsigned int) D.2932; D.2934 = bork (Ty, S); D.2933 = D.2934; return D.2933; # SUCC: EXIT } ;; Function const void* TConvertType(tree_type*) (_Z12TConvertTypeP9tree_type) const void* TConvertType(tree_type*) (type) { const void * Ty; void * D.2926; D.2925; const void * D.2924; # BLOCK 2 # PRED: ENTRY (fallthru) Ty = 0B; D.2925 = type->precision; D.2926 = bork (Ty, D.2925); D.2924 = D.2926; return D.2924; # SUCC: EXIT } the difference is just in whether the representing the bitfield is passed directly to bork as a parameter; when forcibly cast via an unsigned, it gets it right. The generated RTL appears to then select a minimal size for and not take account of argument promotion: ;; Function const void* TConvertType(tree_type*) (_Z12TConvertTypeP9tree_type) ;; Generating RTL for tree basic block 2 ;; Ty = 0B (insn 7 5 0 (set (mem/f/c/i:SI (plus:SI (reg/f:SI 54 virtual-stack-vars) (const_int -4 [0xfffc])) [0 Ty+0 S4 A32]) (const_int 0 [0x0])) -1 (nil) (nil)) ;; D.2925 = type->precision (insn 9 7 11 (set (reg/f:SI 62) (mem/f/c/i:SI (reg/f:SI 53 virtual-incoming-args) [0 type+0 S4 A32])) -1 (nil) (nil)) (insn 11 9 0 (parallel [ (set (reg:HI 59 [ D.2925 ]) (and:HI (mem/s:HI (reg/f:SI 62) [0 S2 A32]) (const_int 511 [0x1ff]))) (clobber (reg:CC 17 flags)) ]) -1 (nil) (nil)) ;; D.2926 = bork (Ty, D.2925) (insn 12 11 13 (set (mem:HI (plus:SI (reg/f:SI 56 virtual-outgoing-args) (const_int 4 [0x4])) [0 S2 A32]) (reg:HI 59 [ D.2925 ])) -1 (nil) (nil)) > I believe I got the TOT -- .svn/entries says "svn://gcc.gnu.org/svn/ > gcc/trunk". Is this a known problem? It's the first I've heard of it, please file a PR. I'm slightly amazed this hasn't been causing bootstrap breakage already, it looks really wrong. cheers, DaveK -- Can't think of a witty .sigline today
Re: Miscompilation...
On Apr 8, 2007, at 11:35 PM, Dave Korn wrote: I believe I got the TOT -- .svn/entries says "svn://gcc.gnu.org/svn/ gcc/trunk". Is this a known problem? It's the first I've heard of it, please file a PR. I'm slightly amazed this hasn't been causing bootstrap breakage already, it looks really wrong. Done: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31513 Thanks for confirming this! -bw