newbie question (fails if optimization is ON)
aleksa
cl /FAcs /O2 source.c ;error
I'm new to C, can someone explain why it doesn't
work as I expect in the second case? Thank you.
// Removes identical (and adjacent) points
// Out: new file size
typedef signed int SINT;
typedef SINT* PSINT;
#define Xflag -1
#define FileEnd 0
SINT RemoveDups (PSINT pFile)
{
PSINT src, dst;
SINT x,y;
src = pFile;
dst = pFile;
loop:
// read XY from src and advance src
x = *src++;
y = *src++;
// write XY to dst and advance dst
*dst++ = x;
*dst++ = y;
// end reached? return new size
if ( (x == Xflag) & (y == FileEnd) )
return ((SINT) dst - (SINT) pFile);
// skip point, if two adjacent points are equal
if ( (x == *src) & (y == *(src+1)) )
src += 2;
goto loop;
}
If I replace the loop with while (1) then
it works in both cases, but I don't understand why
it doesn't work with the goto loop.
(If needed, I'll post the generated listings)
--
comp.lang.c.moderated - moderation address: cl...@plethora.net -- you must
have an appropriate newsgroups line in your header for your mail to be seen,
or the newsgroup name in square brackets in the subject line. Sorry.
Keith Thompson
> cl /FAcs /Od source.c ;OK
> cl /FAcs /O2 source.c ;error
>
> I'm new to C, can someone explain why it doesn't
> work as I expect in the second case? Thank you.
>
> // Removes identical (and adjacent) points
> // Out: new file size
>
> typedef signed int SINT;
> typedef SINT* PSINT;
Style point: these typedefs are a bad idea. Just use "int" and "int*"
directly.
> #define Xflag -1
This is more safely defined as
#define Xflag (-1)
but you don't use it in any way that it makes a difference.
> #define FileEnd 0
>
> SINT RemoveDups (PSINT pFile)
> {
> PSINT src, dst;
This declares both src and dest as pointers to signed int. If you
drop the typedef, keep in mind that the equivalent declaration is:
int *src, *dest;
or, better:
int *src;
int *dest;
> SINT x,y;
>
> src = pFile;
> dst = pFile;
It seems odd to set both src and dst to the same thing. Perhaps it
would make more sense if you explained what you're trying to do.
> loop:
> // read XY from src and advance src
> x = *src++;
> y = *src++;
>
> // write XY to dst and advance dst
> *dst++ = x;
> *dst++ = y;
>
> // end reached? return new size
> if ( (x == Xflag) & (y == FileEnd) )
> return ((SINT) dst - (SINT) pFile);
dst and pFile are pointers. You're converting them to signed ints.
That doesn't make any sense. Just use pointer subtraction;
(dst - pFile) yields a signed integer result (of type ptrdiff_t).
> // skip point, if two adjacent points are equal
> if ( (x == *src) & (y == *(src+1)) )
> src += 2;
>
> goto loop;
> }
Unless I'm missing something, a while(1) loop should be exactly
equivalent to the "loop: ... goto loop;" you have here -- and it would
be *much* better style.
> If I replace the loop with while (1) then
> it works in both cases, but I don't understand why
> it doesn't work with the goto loop.
I don't know what you mean by "doesn't work". You haven't told us
what output you're expecting, what output you got, or how they differ.
You also haven't shown us a program that we can try. I can compile
your function, but I don't know what arguments to pass to it.
Post a small, complete, self-contained program, including a main()
function, that exhibits the problem. Tell us what what the program is
supposed to do, what it actually does, and how they differ.
> (If needed, I'll post the generated listings)
What do yo mean by "generated listings"? If you mean assembly
listings, please don't bother.
--
Keith Thompson (The_Other_Keith) ks...@mib.org <http://www.ghoti.net/~kst>
Nokia
"We must do something. This is something. Therefore, we must do this."
-- Antony Jay and Jonathan Lynn, "Yes Minister"
Ray Kulhanek
You're using the bitwise and (&) operator where you probably want
boolean and (&&). Since boolean true is just any nonzero, that'll
still work if both operands in (a & b) are represented the same
(e.g. as 1), but it's not guaranteed. Apparently in this case, your
compiler will represent both sides the same if optimization is off,
but not if it's on.
And you shouldn't cast dst and pFile to SINT when subtracting them.
It's perfectly legal to subtract two pointers; the result will be of
type size_t (a large unsigned integral type), which is what your
function should probably be returning. But there's no guarantee that
the pointers will fit in an int. I doubt this is the source of your
problem, but you should fix it anyway.
Incidentally, int is signed by default, so the SINT typedef is
superfluous. It's only char that can be either signed or unsigned
unless you specify one.
I'm not sure why it would start working if you replace the label/goto
with while, though. If anything, turning optimization ON would make
the two forms equivalent.
Barry Schwarz
wrote:
Show your while code.
>it works in both cases, but I don't understand why
What are the two cases?
>it doesn't work with the goto loop.
Describe how it doesn't work. What is your input? Output? Expected
output?
--
Remove del for email
James Kuyper
> cl /FAcs /Od source.c ;OK
> cl /FAcs /O2 source.c ;error
>
> I'm new to C, can someone explain why it doesn't
> work as I expect in the second case? Thank you.
It would help enormously if you could explain, in detail, how the way in
which it works in the second case differs from your expectations? In
particular, if it malfunctioned at run time rather than at compile time,
we need to know what inputs you passed to this function, and what
outputs you got from it, both in the "OK" and "error" cases.
> // Removes identical (and adjacent) points
> // Out: new file size
>
> typedef signed int SINT;
Are you aware that "signed int" and "int" are the same type? Is there
any point served by this typedef?
> typedef SINT* PSINT;
Typedefs of pointer types provide little if any benefit, and tend to
lead to errors when people forget that the type is a pointer type.
> #define Xflag -1
> #define FileEnd 0
>
> SINT RemoveDups (PSINT pFile)
> {
> PSINT src, dst;
> SINT x,y;
>
> src = pFile;
> dst = pFile;
>
> loop:
> // read XY from src and advance src
> x = *src++;
> y = *src++;
>
> // write XY to dst and advance dst
> *dst++ = x;
> *dst++ = y;
>
> // end reached? return new size
> if ( (x == Xflag) & (y == FileEnd) )
> return ((SINT) dst - (SINT) pFile);
Converting a pointer to an integer type produces a result that is not
guaranteed to have any particular meaning. If the integer type is large
enough, then converting the integer value back to the same pointer type
is guaranteed to give a pointer equivalent to the original. However,
there's no guarantee that subtracting the two numbers provides any
useful information (such as the distance between the objects pointed at).
There's no guarantee that there is any integer type available that is
big enough to for that reversibility guarantee to apply. However, if
such a type does exist, then in C99, intprt_t and uintprt_t will be
defined by <stdint.h>, to be signed and unsigned types, respectively,
that are big enough to be used for that purpose. You can test for
whether or not those types exist by #including <limits.h> and checking
whether or not INTPTR_MAX or UINTPTR_MAX have been #defined. Note: in
C90, there was no standard way to check whether there was any integer
type big enough for the reversibility guarantee to apply.
On the other hand, if you simply execute the statement
return dst-pFile;
without performing any conversions, you're guaranteed to get a count of
the number of int objects between the two positions pointed at, which
may be what you're actually looking for. The result will be of type
ptrdiff_t, which is defined in <stddef.h> as a signed integer type.
Therefore, I would recommend that you declare RemoveDups() to return
ptrdiff_t.
However, while those are important points to be aware of in general,
they're not likely to be the actual cause of your problem. I don't see
anything else obviously wrong with your code, so I'm not sure why it has
malfunctioned. As indicated above, it would be much easier to debug this
is we knew the way in which it malfunction
> // skip point, if two adjacent points are equal
> if ( (x == *src) & (y == *(src+1)) )
> src += 2;
>
> goto loop;
> }
>
> If I replace the loop with while (1) then
> it works in both cases, but I don't understand why
> it doesn't work with the goto loop.
In general, while() is preferable to goto, but I don't see any reason
why making that change would have any effect. However, it might help if
you post the code with that change made, so we can verify that
conversion to while() was done correctly.
Keith Thompson
> aleksa wrote:
[...]
>> if ( (x == Xflag) & (y == FileEnd) )
>> return ((SINT) dst - (SINT) pFile);
>>
>> // skip point, if two adjacent points are equal
>> if ( (x == *src) & (y == *(src+1)) )
> You're using the bitwise and (&) operator where you probably want
> boolean and (&&). Since boolean true is just any nonzero, that'll
> still work if both operands in (a & b) are represented the same
> (e.g. as 1), but it's not guaranteed. Apparently in this case, your
> compiler will represent both sides the same if optimization is off,
> but not if it's on.
That's not the problem -- or, if it is, either the compiler is broken
or it's doing something very odd in response to undefined behavior.
All the operands of "&" are the results of "==" operators, which by
definition return 0 for false and 1 for true.
Using "&&" rather than "&" certainly makes more sense here, but it's
more a style point than a bug.
> And you shouldn't cast dst and pFile to SINT when subtracting them.
> It's perfectly legal to subtract two pointers; the result will be of
> type size_t (a large unsigned integral type), which is what your
> function should probably be returning.
Pointer subtraction yields a result of type ptrdiff_t, a signed type,
not size_t.
> But there's no guarantee that
> the pointers will fit in an int. I doubt this is the source of your
> problem, but you should fix it anyway.
>
> Incidentally, int is signed by default, so the SINT typedef is
> superfluous. It's only char that can be either signed or unsigned
> unless you specify one.
It's the "signed" keyword that's superfluous because int is signed by
default, not the typedef. The typedef is just a bad idea. It creates
an alias, "SINT", for a type that already has a perfectly good name,
"int" or "signed int". This kind of typedef can make sense if the
name provides some additional information or if the type might vary,
but neither is true here.
And typedefs for pointer types (with the possible exception of
pointer-to-function types) are almost never a good idea.
--
Keith Thompson (The_Other_Keith) ks...@mib.org <http://www.ghoti.net/~kst>
Nokia
"We must do something. This is something. Therefore, we must do this."
-- Antony Jay and Jonathan Lynn, "Yes Minister"
Keith Thompson
> cl /FAcs /Od source.c ;OK
> cl /FAcs /O2 source.c ;error
>
> I'm new to C, can someone explain why it doesn't
> work as I expect in the second case? Thank you.
Suggested reading:
<http://www.catb.org/~esr/faqs/smart-questions.html>
--
Keith Thompson (The_Other_Keith) ks...@mib.org <http://www.ghoti.net/~kst>
Nokia
"We must do something. This is something. Therefore, we must do this."
-- Antony Jay and Jonathan Lynn, "Yes Minister"
Dag-Erling Smørgrav
*what* error?
> I'm new to C, can someone explain why it doesn't work as I expect in
> the second case? Thank you.
Not unless you show us that second case and tell us what input you
provided, what output you expected, and what you actually got.
> SINT RemoveDups (PSINT pFile)
> {
> PSINT src, dst;
> SINT x,y;
>
> src = pFile;
> dst = pFile;
Let's say pFile points to { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, -1, 0 }; on the
first iteration, you get:
> loop:
> // read XY from src and advance src
> x = *src++;
> y = *src++;
x = 0;
y = 1;
src = pFile + 2;
> // write XY to dst and advance dst
> *dst++ = x;
> *dst++ = y;
pFile[0] = 0;
pFile[1] = 1;
dst = pFile + 2;
...so the only effect of these four lines is to increment both src and
dst twice. The contents of pFile are not modified, and the compiler
might even eliminate the *dst assignments altogether since they have no
effect.
> // end reached? return new size
> if ( (x == Xflag) & (y == FileEnd) )
> return ((SINT) dst - (SINT) pFile);
Eventually, you get to a point where x == -1, y == 0 and src == dst ==
pFile + 12, and (style issues aside) your function returns 12.
>
> // skip point, if two adjacent points are equal
> if ( (x == *src) & (y == *(src+1)) )
> src += 2;
This never occurs with my example data, but if it did, the effect of
your function would be (as the name implies) to eliminate consecutive
identical tuples in pFile, such that, for instance,
{ 0, 1, 0, 1, 2, 3, 0, 1, 4, 5, 6, 7, -1, 0 }
becomes
{ 0, 1, 2, 3, 0, 1, 4, 5, 6, 7, -1, 0 }
Note that it does not remove the third instance of { 0, 1 }.
> goto loop;
> }
>
> If I replace the loop with while (1) then it works in both cases, but
> I don't understand why it doesn't work with the goto loop.
There is absolutely no functional difference between the following three
snippets:
/* 1) */
loop:
/* stuff */
goto loop;
/* 2) */
while (1) {
/* stuff */
}
/* 3) */
do {
/* stuff */
} while (1);
other than the scope of declarations within /* stuff */, but there
aren't any in your code.
With optimization disabled, a compiler might generate something like
this:
/* 1) */
loop:
/* stuff */
goto loop;
/* 2) */
loop:
if (1 == 0)
goto end;
/* stuff */
goto loop;
end:
/* 3) */
loop:
/* stuff */
if (1 == 0)
goto loop;
With optimization enabled, the compiler should generate the same code
for the latter two cases as for the first.
DES
--
Dag-Erling Smørgrav - d...@des.no
Francis Glassborow
> "aleksa" <alek...@gmail.com> writes:
>> cl /FAcs /Od source.c ;OK
>> cl /FAcs /O2 source.c ;error
>>
>> I'm new to C, can someone explain why it doesn't
>> work as I expect in the second case? Thank you.
>>
>> // Removes identical (and adjacent) points
>> // Out: new file size
>>
>> typedef signed int SINT;
>> typedef SINT* PSINT;
>
> Style point: these typedefs are a bad idea. Just use "int" and "int*"
> directly.
increases the risk of interference from pre-processor #defines
>
>> #define Xflag -1
>
> This is more safely defined as
> #define Xflag (-1)
All uppercase please
#define XFLAG (-1)
> but you don't use it in any way that it makes a difference.
>
>> #define FileEnd 0
See above, all uppercase please
aleksa
>> boolean and (&&).
Thanks!
The compiler is broken.
After changing to && ("end reached?" and "skip point, if")
the generated code, optimized or not, is always correct (on x86).
---------
RemoveDups is given a list of int XY coords (ie POINT struct).
If two consecutive POINTs are equal (both x and y),
RemoveDups should remove one point, in place.
Finally, it must return the new size of the whole array, in bytes.
Maybe I should change the code so that it uses a POINT struct,
instead of just two consecutive ints?
I suppose that "read XY from src and advance src" would then look like this?
x = *(src.x);
y = *(src.y);
src++;
How do I return the size in bytes if I don't do (SINT) cast?
If I just write (dst - pFile) then I would have to multiply the result.
Since both x86 and ARM (see below) are flat 32bit, casting works good.
// Removes identical (and consecutive) points
// Out: new file size, in bytes
typedef signed int SINT;
typedef SINT* PSINT;
#define Xflag (-1)
#define FileEnd 0
SINT RemoveDups (PSINT pFile)
{
PSINT src, dst;
SINT x,y;
src = pFile;
dst = pFile;
loop:
// while (1) {
// read XY from src and advance src
x = *src++;
y = *src++;
// write XY to dst and advance dst
*dst++ = x;
*dst++ = y;
// end reached? return new size
if ( (x == Xflag) && (y == FileEnd) )
return ((SINT) dst - (SINT) pFile);
// skip point, if two adjacent points are equal
if ( (x == *src) && (y == *(src+1)) )
src += 2;
// }
goto loop;
}
---------
FYI, besides x86, I need this code to execute on ARM also. Both CPUs are 32-bit.
Both compilers had problems with the first version (& instead of &&),
and since I'm new to C, I thought it was my fault (and it was, but..).
ARM compiler said that "loop" will newer be reached (which is not true)
and a x86 compiler didn't say anything, but produced wrong code.
However, the ARM compiler behaves the same even with && corrected,
so I'll post to an ARM-specific forum.
Keith Thompson
>>> You're using the bitwise and (&) operator where you probably want
>>> boolean and (&&).
>> That's not the problem -- or, if it is, either the compiler is broken
I wrote the above. Please don't snip attribution lines.
> Thanks!
> The compiler is broken.
Possible, but unlikely.
> After changing to && ("end reached?" and "skip point, if")
> the generated code, optimized or not, is always correct (on x86).
It's more likely that your program exhibits undefined behavior.
We can't be sure of that, since you've never shown us a complete
program.
[...]
--
Keith Thompson (The_Other_Keith) ks...@mib.org <http://www.ghoti.net/~kst>
Nokia
"We must do something. This is something. Therefore, we must do this."
-- Antony Jay and Jonathan Lynn, "Yes Minister"
aleksa
> We can't be sure of that, since you've never shown us a complete
> program.
I did show a complete program.
That is just a small procedure, to be placed into a library.
I have a rather old CL version (1997):
Microsoft (R) 32-bit C/C++ Optimizing Compiler Version 11.00.7022 for 80x86
Copyright (C) Microsoft Corp 1984-1997. All rights reserved.
If anyone here understands ASM, this is the single & version:
command line: cl /FAcs /O2 source.c
_RemoveDups PROC NEAR ; COMDAT
; 12 : PSINT src, dst;
; 13 : SINT x,y;
; 14 :
; 15 : src = pFile;
; 16 : dst = pFile;
; 17 :
; 18 : loop:
; 19 : // while (1) {
; 20 :
; 21 : // read XY from src and advance src
; 22 : x = *src++;
00000 8b 54 24 04 mov edx, DWORD PTR _pFile$[esp-4]
00004 53 push ebx
00005 55 push ebp
00006 56 push esi
00007 8b 2a mov ebp, DWORD PTR [edx]
00009 8d 72 04 lea esi, DWORD PTR [edx+4]
0000c 8b ce mov ecx, esi
0000e 57 push edi
; 27 : *dst++ = y;
0000f 83 c6 04 add esi, 4
; 28 :
; 29 : // end reached? return new size
; 30 : if ( (x == Xflag) & (y == FileEnd) )
00012 33 c0 xor eax, eax
00014 8b 39 mov edi, DWORD PTR [ecx]
00016 83 c1 04 add ecx, 4
00019 89 7e fc mov DWORD PTR [esi-4], edi
0001c 83 fd ff cmp ebp, -1
0001f 0f 94 c0 sete al
00022 33 db xor ebx, ebx
00024 85 ff test edi, edi
00026 0f 94 c3 sete bl
00029 85 c3 test eax, ebx
0002b 75 42 jne SHORT $L97
$loop$89:
; 32 :
; 33 : // skip point, if two adjacent points are equal
; 34 : if ( (x == *src) & (y == *(src+1)) )
0002d 8b 59 04 mov ebx, DWORD PTR [ecx+4]
00030 33 c0 xor eax, eax
00032 3b fb cmp edi, ebx
00034 8b 39 mov edi, DWORD PTR [ecx]
00036 0f 94 c0 sete al
00039 33 db xor ebx, ebx
0003b 3b ef cmp ebp, edi
0003d 0f 94 c3 sete bl
00040 85 c3 test eax, ebx
00042 74 03 je SHORT $L91
; 35 : src += 2;
00044 83 c1 08 add ecx, 8
$L91:
; 12 : PSINT src, dst;
; 13 : SINT x,y;
; 14 :
; 15 : src = pFile;
; 16 : dst = pFile;
; 17 :
; 18 : loop:
; 19 : // while (1) {
; 20 :
; 21 : // read XY from src and advance src
; 22 : x = *src++;
00047 8b 29 mov ebp, DWORD PTR [ecx]
; 23 : y = *src++;
00049 8b 79 04 mov edi, DWORD PTR [ecx+4]
0004c 83 c1 04 add ecx, 4
; 24 :
; 25 : // write XY to dst and advance dst
; 26 : *dst++ = x;
0004f 89 2e mov DWORD PTR [esi], ebp
00051 83 c6 04 add esi, 4
00054 83 c1 04 add ecx, 4
; 28 :
; 29 : // end reached? return new size
; 30 : if ( (x == Xflag) & (y == FileEnd) )
00057 33 c0 xor eax, eax
00059 89 3e mov DWORD PTR [esi], edi
0005b 83 c6 04 add esi, 4
0005e 83 fd ff cmp ebp, -1
00061 0f 94 c0 sete al
00064 33 db xor ebx, ebx
00066 85 ff test edi, edi
00068 0f 94 c3 sete bl
0006b 85 c3 test eax, ebx
0006d 74 be je SHORT $loop$89
$L97:
; 31 : return ((SINT) dst - (SINT) pFile);
0006f 8b c6 mov eax, esi
; 36 : // }
; 37 : goto loop;
; 38 : }
00071 5f pop edi
00072 5e pop esi
00073 5d pop ebp
00074 2b c2 sub eax, edx
00076 5b pop ebx
00077 c3 ret 0
_RemoveDups ENDP
as you can see, the loop is created twice
(maybe not executed twice, I didn't bother to analyze)
and this is the double && version:
_RemoveDups PROC NEAR ; COMDAT
; 11 : {
00000 56 push esi
00001 57 push edi
; 12 : PSINT src, dst;
; 13 : SINT x,y;
; 14 :
; 15 : src = pFile;
00002 8b 7c 24 0c mov edi, DWORD PTR _pFile$[esp+4]
00006 8b cf mov ecx, edi
; 16 : dst = pFile;
00008 8b c7 mov eax, edi
$loop$89:
; 17 :
; 18 : loop:
; 19 : // while (1) {
; 20 :
; 21 : // read XY from src and advance src
; 22 : x = *src++;
0000a 8b 11 mov edx, DWORD PTR [ecx]
; 23 : y = *src++;
0000c 8b 71 04 mov esi, DWORD PTR [ecx+4]
0000f 83 c1 04 add ecx, 4
; 24 :
; 25 : // write XY to dst and advance dst
; 26 : *dst++ = x;
00012 89 10 mov DWORD PTR [eax], edx
00014 83 c0 04 add eax, 4
00017 83 c1 04 add ecx, 4
; 27 : *dst++ = y;
0001a 89 30 mov DWORD PTR [eax], esi
0001c 83 c0 04 add eax, 4
; 28 :
; 29 : // end reached? return new size
; 30 : if ( (x == Xflag) && (y == FileEnd) )
0001f 83 fa ff cmp edx, -1
00022 75 04 jne SHORT $L90
00024 85 f6 test esi, esi
00026 74 0e je SHORT $L97
$L90:
; 32 :
; 33 : // skip point, if two adjacent points are equal
; 34 : if ( (x == *src) && (y == *(src+1)) )
00028 3b 11 cmp edx, DWORD PTR [ecx]
0002a 75 de jne SHORT $loop$89
0002c 3b 71 04 cmp esi, DWORD PTR [ecx+4]
0002f 75 d9 jne SHORT $loop$89
; 35 : src += 2;
00031 83 c1 08 add ecx, 8
; 36 : // }
; 37 : goto loop;
00034 eb d4 jmp SHORT $loop$89
$L97:
; 31 : return ((SINT) dst - (SINT) pFile);
00036 2b c7 sub eax, edi
; 38 : }
00038 5f pop edi
00039 5e pop esi
0003a c3 ret 0
_RemoveDups ENDP
aleksa
>> cl /FAcs /O2 source.c ;error
>
> *what* error?
I've offered to post the generated listings, but it looks like
no-one would look at that. Finally, I've posted the listings.
> ..and tell us what input you provided..
I did not provide any input, nor actually run the program,
I've just examined the generated code.
> { 0, 1, 0, 1, 2, 3, 0, 1, 4, 5, 6, 7, -1, 0 }
> becomes
> { 0, 1, 2, 3, 0, 1, 4, 5, 6, 7, -1, 0 }
> Note that it does not remove the third instance of { 0, 1 }.
Exactly. That is what I have written and that is what I want.
Dag-Erling Smørgrav
> The compiler is broken.
That is highly unlikely.
> After changing to && ("end reached?" and "skip point, if")
In this particular case, & and && are equivalent.
DES
--
Dag-Erling Smørgrav - d...@des.no
Ike Naar
Keith Thompson <ks...@mib.org> wrote:
>"aleksa" <alek...@gmail.com> writes:
>>
>> // end reached? return new size
>> if ( (x == Xflag) & (y == FileEnd) )
>> return ((SINT) dst - (SINT) pFile);
>
>dst and pFile are pointers. You're converting them to signed ints.
>That doesn't make any sense. Just use pointer subtraction;
>(dst - pFile) yields a signed integer result (of type ptrdiff_t).
((SINT) dst - (SINT) pFILE) does not necessarily make sense and
should not be used; that said, on a typical desktop implementation
it will probably work, and yield the pointer difference measured in bytes.
(dst - pFile) returns the pointer difference measured in SINTs (ints).
I'm not sure what Aleksa wants (the difference in ints seems more
natural to me).
For the difference in bytes, Aleksa could use something like
(char*) dst - (char*) pFile
or (dst - pFile) * sizeof (SINT)
or (dst - pFile) * sizeof *dst
James Kuyper
>>> cl /FAcs /Od source.c ;OK
>>> cl /FAcs /O2 source.c ;error
>> *what* error?
>
> I've offered to post the generated listings, but it looks like
> no-one would look at that. Finally, I've posted the listings.
>
>> ..and tell us what input you provided..
>
> I did not provide any input, nor actually run the program,
> I've just examined the generated code.
OK - then you'll have to explain to us how you know that the generated
code is incorrect.
Keith Thompson
>> It's more likely that your program exhibits undefined behavior.
>> We can't be sure of that, since you've never shown us a complete
>> program.
Once again, I wrote the above.
When you post a followup, your newsreader should automatically insert
an attribution line, such as the
"aleksa" <alek...@gmail.com> writes:
above. Please leave that line in place for any quoted material, so we
can tell who wrote what. It makes the discussion easier to follow.
> I did show a complete program.
I don't believe you did. A complete program would include a main()
function.
> That is just a small procedure, to be placed into a library.
A small procedure is not a complete program. I'm asking for a
program, including a main() function, your RemoveDups() function,
and any additional declarations that might be needed, so that I
can copy-and-paste your program, compile it, and see the results.
> I have a rather old CL version (1997):
> Microsoft (R) 32-bit C/C++ Optimizing Compiler Version 11.00.7022 for 80x86
> Copyright (C) Microsoft Corp 1984-1997. All rights reserved.
>
> If anyone here understands ASM, this is the single & version:
>
[124 lines deleted]
>
>
>
> as you can see, the loop is created twice
> (maybe not executed twice, I didn't bother to analyze)
>
>
>
> and this is the double && version:
>
>
>
>
>
[86 lines deleted]
Sorry, but assembly listing really aren't useful. I don't know x86
assembly very well myself. Even if I did, the C language defines the
behavior of a C program, not the generated code.
Even if you happen to get different generated code for "&" vs. "&&",
the behavior (i.e., the output your program produces) should be
identical.
If you want us to help you debug your program, you need to show us
a complete program and its actual output.
--
Keith Thompson (The_Other_Keith) ks...@mib.org <http://www.ghoti.net/~kst>
Nokia
"We must do something. This is something. Therefore, we must do this."
-- Antony Jay and Jonathan Lynn, "Yes Minister"
Barry Schwarz
<alek...@gmail.com> wrote:
>>> You're using the bitwise and (&) operator where you probably want
>>> boolean and (&&).
>> That's not the problem -- or, if it is, either the compiler is broken
>
>Thanks!
>The compiler is broken.
>
>After changing to && ("end reached?" and "skip point, if")
>the generated code, optimized or not, is always correct (on x86).
>
>---------
>
>RemoveDups is given a list of int XY coords (ie POINT struct).
>If two consecutive POINTs are equal (both x and y),
>RemoveDups should remove one point, in place.
>Finally, it must return the new size of the whole array, in bytes.
>
>Maybe I should change the code so that it uses a POINT struct,
>instead of just two consecutive ints?
>I suppose that "read XY from src and advance src" would then look like this?
> x = *(src.x);
> y = *(src.y);
Unless x and y are pointers, you probably meant (*src).x or the much
easier and more familiar src->x.
--
Remove del for email
Ray Kulhanek
Thanks for the corrections. I really should get in the habit of
rereading the relevant parts of the standard before posting anything.
>> Incidentally, int is signed by default, so the SINT typedef is
>> superfluous. It's only char that can be either signed or unsigned
>> unless you specify one.
>
> It's the "signed" keyword that's superfluous because int is signed by
> default, not the typedef. The typedef is just a bad idea. It creates
> an alias, "SINT", for a type that already has a perfectly good name,
> "int" or "signed int". This kind of typedef can make sense if the
> name provides some additional information or if the type might vary,
> but neither is true here.
I meant the use of the SINT typedef in general, not the
typedef keyword itself.
Ray Kulhanek
(assembly removed for brevity)
> as you can see, the loop is created twice
> (maybe not executed twice, I didn't bother to analyze)
Wait, your problem isn't that the output is bad, or that it's
crashing, but just that the optimized version of the code has some
duplicate code?
That's not a bug; it's an optimization. Here are the general forms
of the code after it's been compiled (long expressions replaced by
expr0, expr1, expr2 for readability). Note how the optimized version
has more statements total, but one less statement *inside the loop*
because it reordered the statements. So the code that needs to
execute once every iteration will run faster, at the expense of an
extra 43 bytes in the binary and a few extra operations before
the loop.
//unoptimized version
loop:
x = *src++; y = *src++; *dst++ = x; *dst++ = y;
if (expr0) goto L1;
if (!expr1) goto L0;
src += 2;
L0: goto loop;
L1: return expr2;
//optimized version
x = *src++; y = *src++; *dst++ = x; *dst++ = y;
if (expr0) goto L1
loop:
if (!expr1) goto L0;
src += 2;
L0: x = *src++; y = *src++; *dst++ = x; *dst++ = y;
if (!expr0) goto loop;
L1: return expr2;
Dag-Erling Smørgrav
> I did show a complete program.
No, you did not. A complete program has a main() function and runs the
malfunctioning code with sample data to demonstrate correct and
incorrect behavior.
DES
--
Dag-Erling Smørgrav - d...@des.no
Dag-Erling Smørgrav
> "Dag-Erling Smørgrav" <d...@des.no> writes:
> > "aleksa" <alek...@gmail.com> writes:
> > > cl /FAcs /Od source.c ;OK
> > > cl /FAcs /O2 source.c ;error
> > *what* error?
> I've offered to post the generated listings, but it looks like
> no-one would look at that. Finally, I've posted the listings.
I don't see how those listings are relevant. This newsgroup is about C,
not about i386 assembly language or Microsoft compilers.
What you *should* have posted, but did not, was sample output showing
that the program *behaves* differently.
If it doesn't, why post here at all?
> > ..and tell us what input you provided..
> I did not provide any input, nor actually run the program,
> I've just examined the generated code.
OK, so what makes you think the generated code is incorrect, and not
just inefficient?
DES
--
Dag-Erling Smørgrav - d...@des.no
Jens Schmidt
> There is absolutely no functional difference between the following three
> snippets:
>
> /* 1) */
> loop:
> /* stuff */
> goto loop;
>
> /* 2) */
> while (1) {
> /* stuff */
> }
>
> /* 3) */
> do {
> /* stuff */
> } while (1);
>
> other than the scope of declarations within /* stuff */, but there
> aren't any in your code.
One further difference:
break and continue statements, if used, would transfer control
to another place in 1 than in 2/3.
--
Greetings,
Jens Schmidt
aleksa
> That's not a bug; it's an optimization.
You are right!
I am sorry and am apologizing to everyone, really, for taking your time.
First, ARM compiler issued a warning "will never be executed",
then CL generated different code for while/goto
and that got me thinking that GOTOs can create problems,
generating invalid code etc.
Again, sorry for taking everyone's time.