Mix of .di and non .di accesses for std::atomic with -mno-volatile-cache/-mvolatile-di

[displaylink-mdomzal]

We have noticed that trying to use std::atomic class in code which is compiled with cache bypass instructions for volatile keyword (-mno-volatile-cache or -mvolatile-di switches depending on the toolchain version) turned on can generate improper machine code. It is visible for ARC600, HS38 (both on 2021.09 baremetal toolchain) and HS58 (prerelease 2022.07 toolchain).

For details see attachment with simple code sample, compilation commands and resulting listing files (also without mentioned above compiler switches for comparison). In short the problem looks like that (based on ARC625 listing as it is simplest to analyze):

 2f2:   da00                    mov_s   r2,0
 2f4:   1bfb b082               stb r2,[fp,-5]                  // Cached write of 0 value to the atomic (fp - 5)
 2f8:   da05                    mov_s   r2,0x5
 2fa:   1bfc b080               st  r2,[fp,-4]
 2fe:   2342 3142               sub r2,fp,0x5                   // r2 = fp - 5
 302:   1200 0883               ldb.di  r3,[r2]                     // Uncached read of the atomic (r3 = uninitialized byte)
 306:   dc01                    mov_s   r12,0x1
 308:   1a00 0322               stb.di  r12,[r2]
 30c:   7a6f                    extb_s  r2,r3                       // r2 = unitialized byte (as r3 is byte already)
 30e:   78e0                    nop_s
 310:   7a4b                    tst_s   r2,r2                       // Set flag based on unitialized value
 312:   f204                    beq_s   8   ;318 <main+0x30>    // Branch based on flag
 314:   da0a                    mov_s   r2,0xa
 316:   f003                    b_s 6   ;31a <main+0x32>
 318:   da0b                    mov_s   r2,0xb
 31a:   7048                    mov_s   r0,r2

Without -mno-volatile-cache/-mvolatile-di the code is generated without .di instructions and works properly. We see similar mix of cached/non cached accesses happening for HS38 and HS58 in the sample (although due to more complicated listing I have not analyzed them for the provided sample).

atomic_issue.zip

[claziss]

-mno-volatile-cache/-mvolatile-di options are made to enable support for load/store instructions usind .di flag, and it does this by emitting .di flag for all variables declared volatile in the C code. The std:atomic_flag is using volatile in its implementation, namely when dealing with __atomic_ buitlins. Thus, using the above compiler flags will trigger emitting .di flags. Also, in std-C a volatile variable resides in the same memory space like a non-volatile variable, which probably conflicts with your understanding of .di flag accesses. But this doesn't show any error in the compiler per-say.

There are several workarounds for this issue, all of them implying compiling your .di special code (the one which requests for -mvolatile-di flag) separately from rest of the code. Here, I am thinking of having a static library, or assembly code.

[displaylink-mdomzal]

Obviously, the issue can be easily workarounded once you are aware of it but the main reason behind opening it is that std::atomic is a part of C++ standard library and it behaves in a surprising way. From my observation most of the people would not guess on first try that -mno-volatile-cache/-mvolatile-di and std::atomic_flag result together in broken code (especially that you can also mark std::atomic_flag as volatile) causing bugs which might be fairly time consuming to investigate. Also ensuring widespread knowledge of this behaviour might be hard to achieve so we would like to address the issue at its root to avoid anyone being surprised in the future.

[claziss]

@displaylink-mdomzal this issue cannot be resolved given the current implementation of -mvolatile-di and the above said about C-standard' volatile understanding.

[displaylink-mdomzal]

Then what about emitting compiler warning/error for this case?

[claziss]

Neither this can be done, as emitting warnings whenever we generate .di flag can break others build. Sorry

[displaylink-mdomzal]

I rather thought about emitting warning when one of std::atomic based classes are instantiated with these compilation flags set. Would that be feasible?

[claziss]

Sorry, no, the only thing which I can do is removing the volatile di option in future releases, and replacing it with a proper implementation.

[displaylink-mdomzal]

So you would provide e.g. some builtins for .di instructions, do I understand it correctly?

[claziss]

That can be done as we speak by any user. I am thinking to implement a special memory space which is separate from the regular one, and sever in this way the unhealthy link between std-c volatile keyword and emitting .di flag. In the new implementation, one may define an uncached variable like this:

__uncached int a;

which will lead to cleaner code and removing the bogus mvolatile-di implementation.