Sign in to follow this  
Genjix

compiler optimizations

Recommended Posts

a = b;
if(a > 0)
   z = a;
a = c;
if(a > 0)
   z = a;

unfortunately analysing compiler output g++ doesn't seem to optimize that. so I tried
a = b;
if(a > 0)
{
   z = a;
}
else
{
   a = c;
   
   if(a > 0)
      z = a;
}

(nesting), but it sacrifices code readability which is a demon. any other suggestions on making something like this look nicer? while avoiding the unnecessary second a calculation when the first expression is true? I was thinking of using recursive functions... clever but not very clear.

Share this post


Link to post
Share on other sites
Did you compile with the optimization flags turned on? I don't know much about compilers, but something as simple as this really ought to be easily caught. In any case, this is another example of needless micro optimization.

EDIT: Read doho's post. I am assuming that the two do in fact do the same thing. I thought about it for a bit then gave up :P

EDIT2: Actually, just read what JohnBolton wrote. Micro-optimization is a waste of time though. [smile]

[Edited by - load_bitmap_file on June 9, 2005 5:49:33 PM]

Share this post


Link to post
Share on other sites
Your hand-optimized version is not the same as the original.

My "optimization" would look like this:

if ( b > 0 )
{
z = b;
}
else if ( c > 0 )
{
z = c;
}
a = c;
But, I just let the compiler worry about it. I can't think of why it wouldn't.

Share this post


Link to post
Share on other sites
int foo(int a, int b, int c)
{
int z;

a = b;
if(a > 0)
z = a;
a = c;
if(a > 0)
z = a;


return z;
}


C:\Temp>cl -c -Oxsy -arch:SSE2 -Fa a.cpp
Microsoft (R) 32-bit C/C++ Optimizing Compiler Version 13.10.3077 for 80x86

	push	ebp
mov ebp, esp
cmp DWORD PTR _b$[ebp], 0
mov eax, DWORD PTR _b$[ebp]
cmovle eax, DWORD PTR _z$[ebp]
cmp DWORD PTR _c$[ebp], 0
cmovg eax, DWORD PTR _c$[ebp]
pop ebp
ret 0


Looks pretty optimimal to me. You could improve it for this particular example by doing frame-pointer omission which is supposed to be included with -Oy. I don't know why MSVC isn't doing that.

Optiming for time rather than size changes the assembly to use branches rather than cmov's. I don't know enough about the nitty-gritty to know which is better. It would probably depend a lot on the contents of the cache at the time.

Share this post


Link to post
Share on other sites
Guest Anonymous Poster
Quote:
Original post by JohnBolton
Your hand-optimized version is not the same as the original.

My "optimization" would look like this:
if ( b > 0 )
{
z = b;
}
else if ( c > 0 )
{
z = c;
}
a = c; But, I just let the compiler worry about it. I can't think of why it wouldn't.


If you're going for the original, I think you mean:
if (c > 0) { z = c; }
else if (b > 0) { z = b; }
a = c;
The second is about as good as it gets for what it does.

MKH has a good thing going if you want to emphasize the assignment more and don't need to modify a.

Share this post


Link to post
Share on other sites
the thing is I forgot to say that the second if should only be evaluated when the first if is false.

a = b;
if(a > 0)
z = a;
a = c;
if(a > 0)
z = a;




unfortunately analysing compiler output g++ doesn't seem to optimize that. so I tried

a = b;
if(a > 0)
{
z = a;
}
else
{
a = c;

if(a > 0)
z = a;
}




This this function is called quite regularly it seemed like such a waste to be checking the second if statement.

Also b and c are gross over simplifications and are in fact complex expressions.

But yeah Magmai Kai Holmlor is the best I think if it's indented. But thanks all for your help. ++

Share this post


Link to post
Share on other sites
Quote:
Original post by Anonymous Poster
If you're going for the original, I think you mean:
if (c > 0) { z = c; }
else if (b > 0) { z = b; }
a = c;

Oops! Another great example of why optimization should be left to the compiler! Optimizing code is a source of bugs.

Share this post


Link to post
Share on other sites
Guest Anonymous Poster
Quote:
Original post by Genjix
the thing is I forgot to say that the second if should only be evaluated when the first if is false.


Hence the "else if" in the suggestions.

Quote:

*** Source Snippet Removed ***

unfortunately analysing compiler output g++ doesn't seem to optimize that. so I tried
*** Source Snippet Removed ***


You do realize that these do different things, right? Let's consider z's value:
if c > 0
then in the first z == c
but in the second z == c if b <= 0 otherwise z == b
if c <= 0
then if b > 0 z == b in both, otherwise z is unchanged in both.

Now, let's consider a's value:
in the first, a == c no matter what.
in the second, if b <= 0 then a == b, otherwise a == c.

Also, g++ won't optimize the first form because it doesn't have the information you have. In particular, it has no way of knowing the second test will fail if the first succeeds (or that the body of the second will produce the same result as the body of the first, or whatever).

Quote:

This this function is called quite regularly it seemed like such a waste to be checking the second if statement.


Ya know, K&R were bright fellows. They saw this problem arising, and put in a keyword just to solve it. "else" ;)

By the way... "seemed like such a waste"? You did profile this before optimizing, right?

Quote:

Also b and c are gross over simplifications and are in fact complex expressions.

But yeah Magmai Kai Holmlor is the best I think if it's indented. But thanks all for your help. ++


Actually, if b and c are more complicated, I'd highly recommend against MKH's solution. It'll get ugly.

Share this post


Link to post
Share on other sites
what i meant by equivalent was

if(x > a)
d = x - a;
else
if(x < b)
d = x - b;


first of all i thought g++ would pick up on this but it clearly didn't (x - a > 0) == (x > a)

if(x < a)
d = x - a;
if(x + p > b + q)
d = (x + p) - (b + q);


you would also think (where p and q are unsigned) that the second expression would be nested. but it isn't.

lastly

if(x - a > 0)
d = x - a;


doesn't seem to be recognised either. (actually I think I'm wrong about this last one).

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this