OSDev.org

Posted: **Thu Dec 17, 2009 12:16 pm**

I don't have optimizations enabled since they appear to trash a simple "if" statement.

   if (tchar == 0x61)
   {
      b_print_string("key was 'a'\n");
   }
   else
   {
      b_print_string("key was not 'a'\n");
   }

Compiling with -O2 causes it to always print the else condition even if I hit the correct key.

-Ian

Posted: **Thu Dec 17, 2009 12:43 pm**

Can you post the code for us to look at?

Posted: **Thu Dec 17, 2009 12:46 pm**

Sure. Here is it:

Code: Select all

// gcc -o testc1.o -c testc1.c -m64 -nostdlib -nostartfiles -nodefaultlibs -fomit-frame-pointer
// ld -T app.ld -o testc1.app testc1.o

void b_print_string(const char *str);
void b_print_char(const char chr);
unsigned char b_input_wait_for_key(void);

int main(void)
{
	unsigned char tchar = 0x65;
	b_print_char(tchar);
	
	b_print_string("Hello world, from C!\nHit a key: ");
	tchar = b_input_wait_for_key();
	
	if (tchar == 'a')
	{
		b_print_string("key was 'a'\n");
	}
	else
	{
		b_print_string("key was not 'a'\n");
	}

	return 0;
}


// C call to os_print_string
// C passes the string address in RDI instead of RSI
void b_print_string(const char *str)
{
	asm volatile ("call 0x00100010" :: "S"(str)); // Make sure string is passed in RSI
}

void b_print_char(const char chr)
{
	asm volatile ("call 0x00100018" :: "a"(chr));
}

// C call to os_input_key_wait
unsigned char b_input_wait_for_key(void)
{
	asm volatile ("call 0x00100038");
}

Looking at the assembly output with -O2 it doesn't even do a compare (cmp).

Thanks,
-Ian

Posted: **Tue Dec 22, 2009 1:51 pm**

The problem might be here :

Code: Select all

// C call to os_input_key_wait
unsigned char b_input_wait_for_key(void)
{
   asm volatile ("call 0x00100038");
}

How can you be sure GCC will keep your return value in rax ? This would probably be better :

Code: Select all

// C call to os_input_key_wait
unsigned char b_input_wait_for_key(void)
{
   unsigned char r;
   asm volatile ("call 0x00100038" : "=a"(r));
   return r;
}

Posted: **Tue Dec 29, 2009 12:00 pm**

Alexis211 wrote:How can you be sure GCC will keep your return value in rax ?

The ABI always has the return value in RAX.

Is there a different way to make this function? (Keystroke is returned in AL)

Code: Select all

Something like:

void b_input_wait_for_key(unsigned char chr)
{
	asm volatile ("call 0x00100038" : "=a"(chr));
}

Instead of: (gives a "warning: control reaches end of non-void function" error)

unsigned char b_input_wait_for_key(void)
{
	asm volatile ("call 0x00100038");
}

Posted: **Tue Dec 29, 2009 1:00 pm**

The first code block loads a result into a stack register, which consequently gets ignored.
The second code block loads *AX, but the compiler can still decide to change it before returning from the call. In fact, GCC may inline the function for optimisation purposes, and you'll never see the result (more likely, your code will crash due to registers being altered without you telling the compiler about it. Volatile or not.)

Not to mention, both methods will fail when you'd try to use arguments later.

Posted: **Tue Dec 29, 2009 1:27 pm**

They should be

Code: Select all

void b_input_wait_for_key(unsigned char chr)
{
   asm volatile ("call 0x00100038" : "=a"(chr) : "0" (chr));
   return chr;
}

unsigned char b_input_wait_for_key(void)
{
   unsigned char chr;
   asm volatile ("call 0x00100038" : "=a" (chr));
   return chr;
}

The first one says "The value chr is modified and the value returned in the appropriate portion of register A, and store the starting value of chr in the same register" (The "0" means register index 0, i.e, the first one you specify). The second one should be reasonably obvious.

The code generated should be identical to how it would be otherwise; though I'd want to let GCC optimize everything by inlining.

Of course, for the first one, I'm assuming you want the parameter in RAX. If you actually want it in RSI, then tell GCC that.

If your code functions follow the x86_64 ABI, then the best method is to just declare the functions, and have an ASM file like

Code: Select all

.global some_function
some_function .equ absolute_address_of_function

Assuming that won't violate the maximum PC-relative displacement rules

Remember: If you don't tell GCC what registers your putting things in, it doesn't know!Unlike compilers such as Visual C++, it's actually pretty smart at dealing with ASM blocks (Visual C++'s handling of them is horrible; it basically turns off the optimizer for the function). Of course, this smartness means complexity.

Also remember to always give it the clobber list if needed.

Posted: **Tue Dec 29, 2009 4:37 pm**

ReturnInfinity wrote:Sure. Here is it:

Code: Select all

// gcc -o testc1.o -c testc1.c -m64 -nostdlib -nostartfiles -nodefaultlibs -fomit-frame-pointer
// ld -T app.ld -o testc1.app testc1.o

void b_print_string(const char *str);
void b_print_char(const char chr);
unsigned char b_input_wait_for_key(void);

int main(void)
{
	unsigned char tchar = 0x65;
	b_print_char(tchar);
	
	b_print_string("Hello world, from C!\nHit a key: ");
	tchar = b_input_wait_for_key();
	
	if (tchar == 'a')
	{
		b_print_string("key was 'a'\n");
	}
	else
	{
		b_print_string("key was not 'a'\n");
	}

	return 0;
}

Thanks,
-Ian

Hi: the solution is very simple. GCC has optimized your code since as far as it can tell, the value of tchar cannot change.

1. tchar is set to 0x65, the character 'e' in the ascii table.
2. You then call a function to wait for a key. But this function does not have a return statement.
3. GCC takes this to mean that there is no way that your wait key function could change the value of 'tchar'.
4. Gcc sees this as an opportunity to optimize a 'cmp', thus eliminating one possible cache miss, and also use a hardcoded value.
5. With the assumption that tchar will always, so far as GCC can see, be equal to 0x65, it optimizes away your whole 'if' construct, and just calls the obvious solution: if 'tchar' cannot change, then obviously the 'else' will always be taken.

Result: you get the optimized away result of the GCC's pre-coded comparison of 'tchar' against 0x61, or 'a'.

Solution:

1. Place a return statement inside of the wait key function.
2. This is the correct solution, actually: make 'tchar' a volatile char. GCC will be clued in to the fact that it can change without any explicit or seen implicit input from files it is able to see, and thus not pre-compute and optimize anything that references it.

The last note to take is that you should read up properly on optimizations before trying to use them. If you can't think about what the compiler is doing, then you will fail to get results.

--All the best in your project
gravaera

Posted: **Tue Dec 29, 2009 5:49 pm**

Erm, no. Making tchar volatile won't help here.

Whats happening is GCC is inlining b_input_wait_for_key. This function contains an assembly instruction with no apparent side effects. GCC then procedes to assign tchar to a register thats not AL, and all hell breaks loose.

Basically, when GCC says "No return value for function returning non-void", what it's really saying is "The return value is gonna be junk. Whatever you did, I'm assuming you called a function like abort which never returns without telling me about that never returning, but I don't know, so I'm going to play it safe". When it inlines that code and then does the assignment, it goes "Erm, stuck! don't know what to do!" and throws away the assignment.

The correct thing to do is notify the compiler of return values properly. Putting a volatile there will just make the compiler throw up it's hands in the year.

Years of experience programming have shown me one thing: There is never a reason for a variable to be declared volatile. I'm being serious, never.

Posted: **Tue Dec 29, 2009 7:03 pm**

Owen wrote:Erm, no. Making tchar volatile won't help here.

Let's wait and see.

Posted: **Tue Dec 29, 2009 7:06 pm**

Basically, when GCC says "No return value for function returning non-void", what it's really saying is "The return value is gonna be junk.

Maybe, just maybe, GCC is saying that the function is defined as returning a non-void type, and it doesn't have a return statement in there at all. I dunno, maybe the compiler doesn't know how to say what it wants to say or something.

Especially when you see the lack of a return statement in the function. Gee, I wonder if GCC is actually saying "There's no return value" rather than "The return value is going to be junk":

Code: Select all

unsigned char b_input_wait_for_key(void)
{
   asm volatile ("call 0x00100038");
}

Use a function pointer if all you're doing is calling a function with the inline assembly:

Code: Select all

typedef unsigned char (*func)(void);
func b_input_wait_for_key = (func) 0x100038;

Years of experience programming have shown me one thing: There is never a reason for a variable to be declared volatile. I'm being serious, never.

While I agree with you in part, I'd like to know how you'd approach a global variable that's affected by external state changes - eg, a mutex that gets unlocked by an IRQ.

Posted: **Tue Dec 29, 2009 8:28 pm**

pcmattman wrote:
Years of experience programming have shown me one thing: There is never a reason for a variable to be declared volatile. I'm being serious, never.
While I agree with you in part, I'd like to know how you'd approach a global variable that's affected by external state changes - eg, a mutex that gets unlocked by an IRQ.

Or what about a location in memory that gets changed by hardware?

Posted: **Wed Dec 30, 2009 3:24 am**

Owen wrote:Years of experience programming have shown me one thing: There is never a reason for a variable to be declared volatile. I'm being serious, never.

You sound like a linux evangelist - this is not the linux kernel where all instances of the volatile keyword have disappeared into assembly stubs because people didn't want to see volatile... </countertroll>

I'll have to agree that declaring tchar volatile in this context is semantically nonsense. As well as volatile often being used for the wrong thing.

Owen wrote:Erm, no. Making tchar volatile won't help here.
Let's wait and see.

If it does work, it's a really ugly hack.

Posted: **Wed Dec 30, 2009 5:39 am**

Yah. I'll take back my little witty comeback then. Sorry if I was misleading the OP with the volatile assertion

--All the best
gravaera

Posted: **Wed Dec 30, 2009 6:55 am**

Firestryke31 wrote:
pcmattman wrote:
Years of experience programming have shown me one thing: There is never a reason for a variable to be declared volatile. I'm being serious, never.
While I agree with you in part, I'd like to know how you'd approach a global variable that's affected by external state changes - eg, a mutex that gets unlocked by an IRQ.
Or what about a location in memory that gets changed by hardware?

GCC's __builtin_atomic (Or MVSC's Interlocked*)

Actually, there is one case where I will use volatile: Inside a pair of macros:

Code: Select all

#define ATOMIC_LOAD(_type, _addr) (*((volatile _type*) _addr))
#define ATOMIC_STORE(_type, _addr, _val) (*((volatile type*) _addr) = _val)

(Alternatively, in C++ you can make this nicer and use templates, creating an "atomic" class).

The reason? When you use volatile, operating on an atomic variable looks like operating on a normal one - even though the semantics are very different. It is far too easy to make mistakes. In general, I prefer the explicit approach to doing things that are different from the normal.

gravaera wrote:
Owen wrote:Erm, no. Making tchar volatile won't help here.
Let's wait and see.

tchar will never have an address, because it's address is never taken, and it will be kept in registers. So GCC will shrug and go "Well, that was pointless". Basically, any access of tchar from outside that function would break aliasing rules

OSDev.org

Linking a C program to an Assembly OS's system calls

Re: Linking a C program to an Assembly OS's system calls

Re: Linking a C program to an Assembly OS's system calls

Re: Linking a C program to an Assembly OS's system calls

Re: Linking a C program to an Assembly OS's system calls

Re: Linking a C program to an Assembly OS's system calls

Re: Linking a C program to an Assembly OS's system calls

Re: Linking a C program to an Assembly OS's system calls

Re: Linking a C program to an Assembly OS's system calls

Re: Linking a C program to an Assembly OS's system calls

Re: Linking a C program to an Assembly OS's system calls

Re: Linking a C program to an Assembly OS's system calls

Re: Linking a C program to an Assembly OS's system calls

Re: Linking a C program to an Assembly OS's system calls

Re: Linking a C program to an Assembly OS's system calls

Re: Linking a C program to an Assembly OS's system calls