Skip to main content
Code Review

Return to Answer

replaced http://stackoverflow.com/ with https://stackoverflow.com/
Source Link

This works because bitwise XORing any value with itself gives you back 0. The advantage is that it is shorter (2 bytes instead of 5 bytes) and faster faster.

This works because bitwise XORing any value with itself gives you back 0. The advantage is that it is shorter (2 bytes instead of 5 bytes) and faster.

This works because bitwise XORing any value with itself gives you back 0. The advantage is that it is shorter (2 bytes instead of 5 bytes) and faster.

deleted 122 characters in body
Source Link
Cody Gray
  • 4.6k
  • 19
  • 30

In some cases, conditional moves will make the code faster by eliminating the possibility of branch mispredictions. In this case, you won't see any performance improvement, but these instructions are still something to be aware of and this does make the code slightly easier to read. (Assuming that you want to keep the dependency on the ebp register, which might be handy if you want to make the upper limit variable. Otherwise, just hard-code the 1000000 and use the first version. Better yet, define a constant for that upper bound so you can give it a readable name.)

I can tell that you're working with signed integer values because you're using the l and g condition codes. If you were working with unsigned integer values, you'd be using the b and a condition codes. Therefore, you should be doing a signed integer multiply (imul instead of mul). In fact, imul is preferable for a number of reasons, chief among them is the fact that isit has a normal, two-operand form that works just like any other instruction, which means you don't have to worry about the hard-coded register operands that mul requires. This makes the code easier to optimize and, frankly, easier to read. You can use imul for unsigned multiplications, too, as long as you aren't worried about overflow, since the lower 32 bits are always the same. (And clearly you aren't worried about that here, since your print_eax function used to display the output can only print 32-bit values!)

start:
 ; Provided by teacher: reads in a hexadecimal number from stdin,
 ; returning it in EAX.
 call read_hex
 ; Check for underflow: if the input value is less than or equal to 0,
 ; we skip the loop altogether.
 ; NOTE: The naive way to do this check is "cmp eax, 0", but as with
 ; the XOR trick discussed above, "test eax, eax" is both
 ; shorter and faster, so that's what everyone uses when they
 ; want to test the value of a register against 0.
 ; (You still use CMP with memory operands, of course.)
 test eax, eax
 jle finished
 ; Check for overflow: if the input value is larger than 1000000,
 ; we bound it to be exactly 1000000.
 cmp eax, 1000000
 mov edx, 1000000
 cmovg eax, edx
 ; Prepare to enter the loop:
 mov edx, eax ; make a copy of the input
 mov ecx, 1 ; initialize our counter
printSquares:
 mov eax, ecx ; put the counter into our scratch register, EAX
 imul eax, eax ; EAX = EAX * EAX
 ; Print the square (in EAX) by calling teacher-provided function,
 ; which prints the contents of EAX to stdout.
 ; It may or may not clobber EAX (we don't care),
 ; but it does not return a significant value.
 ; TODO: Make sure it doesn't clobber ECX or EDX!
 call print_eax
 inc ecx ; increment our counter
 ; See if we've reached the input value yet.
 ; If not, continue looping.
 cmp ecx, edx
 jle printSquares
finished:
 ; We're now done with the loop, so exit.
 push 0
 call [ExitProcess]

Warning: There is still potentially a bug in here, depending on how the print_eax function behaves. Normally, in most calling conventions, a function is allowed to clobber the eax, ecx, and edx registers. Therefore, to be on the safe side, you would have to assume that print_eax could clobber the values in these registers, meaning that you would have to either pick different registers to store your temporary values in, or you would have to explicitly save the contents of these registers before calling print_eax and restore them afterwards. However, if you know from the documentation or implementation of print_eax that it does not clobber either ecx or edx, then you are safe. Your original code had this bug, too, yet it worked properly, so I assume this is safe.

In some cases, conditional moves will make the code faster by eliminating the possibility of branch mispredictions. In this case, you won't see any performance improvement, but these instructions are still something to be aware of and this does make the code slightly easier to read. (Assuming that you want to keep the dependency on the ebp register, which might be handy if you want to make the upper limit variable. Otherwise, just hard-code the 1000000 and use the first version.)

I can tell that you're working with signed integer values because you're using the l and g condition codes. If you were working with unsigned integer values, you'd be using the b and a condition codes. Therefore, you should be doing a signed integer multiply (imul instead of mul). In fact, imul is preferable for a number of reasons, chief among them is the fact that is has a normal, two-operand form that works just like any other instruction, which means you don't have to worry about the hard-coded register operands that mul requires. This makes the code easier to optimize and, frankly, easier to read. You can use imul for unsigned multiplications, too, as long as you aren't worried about overflow, since the lower 32 bits are always the same. (And clearly you aren't worried about that here, since your print_eax function used to display the output can only print 32-bit values!)

start:
 ; Provided by teacher: reads in a hexadecimal number from stdin,
 ; returning it in EAX.
 call read_hex
 ; Check for underflow: if the input value is less than or equal to 0,
 ; we skip the loop altogether.
 ; NOTE: The naive way to do this check is "cmp eax, 0", but as with
 ; the XOR trick discussed above, "test eax, eax" is both
 ; shorter and faster, so that's what everyone uses when they
 ; want to test the value of a register against 0.
 ; (You still use CMP with memory operands, of course.)
 test eax, eax
 jle finished
 ; Check for overflow: if the input value is larger than 1000000,
 ; we bound it to be exactly 1000000.
 cmp eax, 1000000
 mov edx, 1000000
 cmovg eax, edx
 ; Prepare to enter the loop:
 mov edx, eax ; make a copy of the input
 mov ecx, 1 ; initialize our counter
printSquares:
 mov eax, ecx ; put the counter into our scratch register, EAX
 imul eax, eax ; EAX = EAX * EAX
 ; Print the square (in EAX) by calling teacher-provided function,
 ; which prints the contents of EAX to stdout.
 ; It may or may not clobber EAX (we don't care),
 ; but it does not return a significant value.
 call print_eax
 inc ecx ; increment our counter
 ; See if we've reached the input value yet.
 ; If not, continue looping.
 cmp ecx, edx
 jle printSquares
finished:
 ; We're now done with the loop, so exit.
 push 0
 call [ExitProcess]

Warning: There is still potentially a bug in here, depending on how the print_eax function behaves. Normally, in most calling conventions, a function is allowed to clobber the eax, ecx, and edx registers. Therefore, to be on the safe side, you would have to assume that print_eax could clobber the values in these registers, meaning that you would have to either pick different registers to store your temporary values in, or you would have to explicitly save the contents of these registers before calling print_eax and restore them afterwards. However, if you know from the documentation or implementation of print_eax that it does not clobber ecx, then you are safe. Your original code had this bug, too, yet it worked properly, so I assume this is safe.

In some cases, conditional moves will make the code faster by eliminating the possibility of branch mispredictions. In this case, you won't see any performance improvement, but these instructions are still something to be aware of and this does make the code slightly easier to read. (Assuming that you want to keep the dependency on the ebp register, which might be handy if you want to make the upper limit variable. Otherwise, just hard-code the 1000000 and use the first version. Better yet, define a constant for that upper bound so you can give it a readable name.)

I can tell that you're working with signed integer values because you're using the l and g condition codes. If you were working with unsigned integer values, you'd be using the b and a condition codes. Therefore, you should be doing a signed integer multiply (imul instead of mul). In fact, imul is preferable for a number of reasons, chief among them is the fact that it has a normal, two-operand form that works just like any other instruction, which means you don't have to worry about the hard-coded register operands that mul requires. This makes the code easier to optimize and, frankly, easier to read. You can use imul for unsigned multiplications, too, as long as you aren't worried about overflow, since the lower 32 bits are always the same. (And clearly you aren't worried about that here, since your print_eax function used to display the output can only print 32-bit values!)

start:
 ; Provided by teacher: reads in a hexadecimal number from stdin,
 ; returning it in EAX.
 call read_hex
 ; Check for underflow: if the input value is less than or equal to 0,
 ; we skip the loop altogether.
 ; NOTE: The naive way to do this check is "cmp eax, 0", but as with
 ; the XOR trick discussed above, "test eax, eax" is both
 ; shorter and faster, so that's what everyone uses when they
 ; want to test the value of a register against 0.
 ; (You still use CMP with memory operands, of course.)
 test eax, eax
 jle finished
 ; Check for overflow: if the input value is larger than 1000000,
 ; we bound it to be exactly 1000000.
 cmp eax, 1000000
 mov edx, 1000000
 cmovg eax, edx
 ; Prepare to enter the loop:
 mov edx, eax ; make a copy of the input
 mov ecx, 1 ; initialize our counter
printSquares:
 mov eax, ecx ; put the counter into our scratch register, EAX
 imul eax, eax ; EAX = EAX * EAX
 ; Print the square (in EAX) by calling teacher-provided function,
 ; which prints the contents of EAX to stdout.
 ; It may or may not clobber EAX (we don't care),
 ; but it does not return a significant value.
 ; TODO: Make sure it doesn't clobber ECX or EDX!
 call print_eax
 inc ecx ; increment our counter
 ; See if we've reached the input value yet.
 ; If not, continue looping.
 cmp ecx, edx
 jle printSquares
finished:
 ; We're now done with the loop, so exit.
 push 0
 call [ExitProcess]

Warning: There is still potentially a bug in here, depending on how the print_eax function behaves. Normally, in most calling conventions, a function is allowed to clobber the eax, ecx, and edx registers. Therefore, to be on the safe side, you would have to assume that print_eax could clobber the values in these registers, meaning that you would have to either pick different registers to store your temporary values in, or you would have to explicitly save the contents of these registers before calling print_eax and restore them afterwards. However, if you know from the documentation or implementation of print_eax that it does not clobber either ecx or edx, then you are safe. Your original code had this bug, too, yet it worked properly, so I assume this is safe.

deleted 122 characters in body
Source Link
Cody Gray
  • 4.6k
  • 19
  • 30

I can tell that you're working with signed integer values because you're using the l and g condition codes. If you were working with unsigned integer values, you'd be using the b and a condition codes. Therefore, you should be doing a signed integer multiply (imul instead of mul). In fact, imul is preferable for a number of reasons, chief among them is the fact that is has a normal, two-operand form that works just like any other instruction, which means you don't have to worry about the hard-coded register operands that mul requires. This makes the code easier to optimize and, frankly, easier to read. You can use imul for unsigned multiplications, too, as long as you aren't worried about overflow—and hereoverflow, since the lower 32 bits are always the same. (And clearly you aren't worried about that here, because 10000002 issince your maximum possible value, and it fits easily into aprint_eax function used to display the output can only print 32-bit integer.values!)

Warning: There is still potentially a bug in here, depending on how the print_eax function behaves. Normally, in most calling conventions, a function is allowed to clobber the eax, ecx, and edx registers. Therefore, to be on the safe side, you would have to assume that print_eax could clobber the values in these registers, meaning that you would have to either pick different registers to store your temporary values in, or you would have to explicitly save the contents of these registers before calling print_eax and restore them afterwards. However, if you know from the documentation or implementation of print_eax that it does not clobber ecx, then you are safe. Your original code had this bug, too, yet it worked properly, so I assume this is safe.

I can tell that you're working with signed integer values because you're using the l and g condition codes. If you were working with unsigned integer values, you'd be using the b and a condition codes. Therefore, you should be doing a signed integer multiply (imul instead of mul). In fact, imul is preferable for a number of reasons, chief among them is the fact that is has a normal, two-operand form that works just like any other instruction, which means you don't have to worry about the hard-coded register operands that mul requires. This makes the code easier to optimize and, frankly, easier to read. You can use imul for unsigned multiplications, too, as long as you aren't worried about overflow—and here you aren't, because 10000002 is your maximum possible value, and it fits easily into a 32-bit integer.

I can tell that you're working with signed integer values because you're using the l and g condition codes. If you were working with unsigned integer values, you'd be using the b and a condition codes. Therefore, you should be doing a signed integer multiply (imul instead of mul). In fact, imul is preferable for a number of reasons, chief among them is the fact that is has a normal, two-operand form that works just like any other instruction, which means you don't have to worry about the hard-coded register operands that mul requires. This makes the code easier to optimize and, frankly, easier to read. You can use imul for unsigned multiplications, too, as long as you aren't worried about overflow, since the lower 32 bits are always the same. (And clearly you aren't worried about that here, since your print_eax function used to display the output can only print 32-bit values!)

Warning: There is still potentially a bug in here, depending on how the print_eax function behaves. Normally, in most calling conventions, a function is allowed to clobber the eax, ecx, and edx registers. Therefore, to be on the safe side, you would have to assume that print_eax could clobber the values in these registers, meaning that you would have to either pick different registers to store your temporary values in, or you would have to explicitly save the contents of these registers before calling print_eax and restore them afterwards. However, if you know from the documentation or implementation of print_eax that it does not clobber ecx, then you are safe. Your original code had this bug, too, yet it worked properly, so I assume this is safe.

Source Link
Cody Gray
  • 4.6k
  • 19
  • 30
Loading
lang-lisp

AltStyle によって変換されたページ (->オリジナル) /