On Sun, 3 Apr 2022 at 09:38, Andrew Pinski <pins...@gmail.com> wrote: > > On Fri, Apr 1, 2022 at 10:24 AM Mark Rutland via Gcc <gcc@gcc.gnu.org> wrote: > > > > Hi Jeremy, > > > > Thanks for raising this. > > > > On Fri, Apr 01, 2022 at 11:44:06AM -0500, Jeremy Linton wrote: > > > The relaxed variants of read/write macros are only declared > > > as `asm volatile()` which forces the compiler to generate the > > > instruction in the code path as intended. The only problem > > > is that it doesn't also tell the compiler that there may > > > be memory side effects. Meaning that if a function is comprised > > > entirely of relaxed io operations, the compiler may think that > > > it only has register side effects and doesn't need to be called. > > > > As I mentioned on a private mail, I don't think that reasoning above is > > correct, and I think this is a miscompilation (i.e. a compiler bug). > > > > The important thing is that any `asm volatile` may have a side effects > > generally outside of memory or GPRs, and whether the assembly contains a > > memory > > load/store is immaterial. We should not need to add a memory clobber in > > order > > to retain the volatile semantic. > > > > See: > > > > https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html#Volatile > > > > ... and consider the x86 example that reads rdtsc, or an arm64 sequence > > like: > > > > | void do_sysreg_thing(void) > > | { > > | unsigned long tmp; > > | > > | tmp = read_sysreg(some_reg); > > | tmp |= SOME_BIT; > > | write_sysreg(some_reg); > > | } > > > > ... where there's no memory that we should need to hazard against. > > > > This patch might workaround the issue, but I don't believe it is a correct > > fix. > > It might not be the most restricted fix but it is a fix. > The best fix is to tell that you are writing to that location of memory. > volatile asm does not do what you think it does. > You didn't read further down about memory clobbers: > https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html#Clobbers-and-Scratch-Registers > Specifically this part: > The "memory" clobber tells the compiler that the assembly code > performs memory reads or writes to items other than those listed in > the input and output operands >
So should we be using "m"(*addr) instead of "r"(addr) here? (along with the appropriately sized casts)
