Optimization bug of range comparison - by Gratian Lup

Status : 

  Duplicate<br /><br />
		This item appears to be a duplicate of another existing Connect or internal item.<br /><br />
		A more detailed explanation for the resolution of this particular item may have been provided in the comments section.

Sign in
to vote
ID 780362 Comments
Status Closed Workarounds
Type Bug Repros 0
Opened 3/1/2013 3:18:30 AM
Access Restriction Public


I encountered a bug when performing a range comparison (upper-case/lower-case for characters) in Release mode (with optimizations).  
The bug seems to be related to the optimization of the range comparisons 
(2 cmp + 1 and => 1 sub + 1 unsigned cmp), inlining and redundant load elimination from a char string.  

The code is as follows:  
static bool cp_is_uppercase_letter(char letter) {  
	return (letter >= 'A') && (letter <= 'Z');  
static bool cp_is_valid_letter(char letter, bool match_lowercase) {  
	return cp_is_uppercase_letter(letter) ||  
		   (match_lowercase && cp_is_lowercase_letter(letter));  
static bool cp_letters_have_same_case(char a, char b) {  
        return cp_is_lowercase_letter(a) == cp_is_lowercase_letter(b);  
bool cp_is_valid_double_letter(cp_token_t* token, bool match_lowercase) {  
        return cp_is_word(*token) &&   
		   (token->length == 2) &&  
		   cp_is_valid_letter(token->word[0], match_lowercase) &&  
		   cp_is_valid_letter(token->word[1], match_lowercase) &&  
		   cp_letters_have_same_case(token->word[0], token->word[1]) &&  
		   cp_letters_are_in_range(token->word[0], token->word[1]);  
All calls except "cp_letters_are_in_range" are inlined and the problem is from the code  
found in "cp_letters_have_same_case". As it can be seen from the assembly, for the first  letter (token->word[0]) a valid range comparison is generated, while for the second one a strange sequence of instructions:  
mov eax, 19h    // 'Z' - 'A'  
cmp ah, al         // Always compares 0 with 19h  
It should have been 'cmp al, X',   
where X is a register containing the second letter - 'A'.  

I isolated the problem and attached the project and an image with the assembly  
and some annotations explaining what I found wrong with the generated instructions. 

I observed that there are three "solutions":  
1. Load the two letters before using them (manual redundant load elimination)  

2. Write the tests as a series of "if"s and put a "printf" between "cp_letters_have_same_case"
    and "cp_letters_are_in_range'. This prevents the redundant load elimination, probably
    because alias analysis thinks that "printf" might modify the token.
    Probably any external function would be as good as the "printf".

3. Rewrite the code from "cp_letters_have_same_case" as follows:
    bool result1 = cp_is_lowercase_letter(a);
    bool result2 = cp_is_lowercase_letter(b);
    return result1 == result2;

    This (strangely) generates very different code not affected by the bug.

I am using Visual Studio Express 2012 Desktop Version 11.0.51106.01 Update 1
Optimization settings for the project:

Optimization: Maximize Speed (/O2)
Inline Function Expansion: Default
Enable Instrinsic Functions: Yes (/Oi)
Favor Size or Speed: Neither
Whole Program Optimization: No
Enable C++ Exceptions: No
Runtime Library: Multi-threaded (/MT)

Best regards,
Gratian Lup
Sign in to post a comment.
Posted by Gratian Lup on 7/9/2013 at 4:52 AM
The bug has been solved in Visual Studio 2013.
Posted by Microsoft on 3/6/2013 at 2:52 PM
Hi, thanks for the bug report. This is a bug in the compiler optimizer and will be fixed in a future release.

It looks like you already have a workaround for this issue. If you feel you would like a hotfix for VS 2012, please visit the support site: http://support.microsoft.com/common/international.aspx?RDPATH=dm;en-us;selectassist&target=assistance

I am closing this MSConnect item. Feel free to re-activate it if you need anything else.

Eric Brumer
Microsoft Visual C++ Team
Posted by Microsoft on 3/3/2013 at 9:22 PM
Thanks for your feedback.

We are rerouting this issue to the appropriate group within the Visual Studio Product Team for triage and resolution. These specialized experts will follow-up with your issue.
Posted by Microsoft on 3/1/2013 at 3:50 AM
Thank you for your feedback, we are currently reviewing the issue you have submitted. If this issue is urgent, please contact support directly(http://support.microsoft.com)