Re: [edk2-devel] [PATCH] UefiCpuPkg: add volatile qualifier to page table related variable

["Laszlo Ersek" <lersek@redhat.com>]

On 2/22/24 11:23, Ni, Ray wrote:
>>> I agree with the idea (I think it's a necessary change, or put
>>> differently, an improvement, even though I may not be convinced that it
>>> is a *sufficient* improvement; but let's not rehash all that here
>>> again); however, I think the implementation is not the greatest.
>>>
>>> Volatile-qualifying the local variables does not seem useful for
>>> anything. It's fine -- actually: it's beneficial -- if the compiler
>>> optimizes accesses to those locals -- being on the stack -- as heavily
>>> as it can. In other words, those parts of the patch look like a small
>>> performance regression.
> 
> I did experiment using MSVC compiler with below code:
>   int main () {
>     int x;
>     x = 3;
>     return 0;
>   }
> 
> If building the above code in optimized mode, the disassembly does not contain
> any reference to local variable x.
> 
> But if I changed "int x" to "volatile int x", the compiler does not optimize out the
> assignment of x.
> 
> So, it means the "volatile"  matters even when it applies local variables.

Yes, but that's beside the point. It does not matter what kinds of
accesses the compiler generates for the local variables, as those
variables exist on the stack, and are not seen by anything else
(certainly not by other processors). The accesses that matter are to the
page tables, in dynamically allocated memory, so if anything, *those*
accesses should be "volatile".

Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115924): https://edk2.groups.io/g/devel/message/115924
Mute This Topic: https://groups.io/mt/104483610/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-