On 16.10.2018 00:46, Russell King - ARM Linux wrote: > On Tue, Oct 16, 2018 at 12:39:54AM +0200, Stefan Agner wrote: >> On 16.10.2018 00:23, Russell King - ARM Linux wrote: >> > On Tue, Oct 16, 2018 at 12:16:29AM +0200, Stefan Agner wrote: >> >> When functions incoming parameters are not in input operands list gcc >> >> 4.5 does not load the parameters into registers before calling this >> >> function but the inline assembly assumes valid addresses inside this >> >> function. This breaks the code because r0 and r1 are invalid when >> >> execution enters v4wb_copy_user_page () >> > >> > NAK. Naked functions must never be inlined. Please add a "noinline" >> > attribute to the function rather than making things more complex. >> > >> >> To be honest, I did not put much thought into this commit since it is >> just doing to copypage-fa.c what 9a40ac86152c ("ARM: 6164/1: Add kto and >> kfrom to input operands list.") has been done to the other copypage >> implementations... >> >> [adding Khem] >> >> > The GCC manual states: >> > >> > `naked' >> > Use this attribute on the ARM, AVR, MCORE, MSP430, NDS32, RL78, RX >> > and SPU ports to indicate that the specified function does not >> > need prologue/epilogue sequences generated by the compiler. It is >> > up to the programmer to provide these sequences. The only >> > ^^^^^^^^ >> > statements that can be safely included in naked functions are >> > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ >> > `asm' statements that do not have operands. All other statements, >> > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ >> > including declarations of local variables, `if' statements, and so >> > forth, should be avoided. Naked functions should be used to >> > implement the body of an assembly function, while allowing the >> > compiler to construct the requisite function declaration for the >> > assembler. >> > >> > The 'I' attribute is fine here because it is a constant that is not >> > allowed to be in a register (and hence has no code generation side >> > effects.) >> > >> > Adding operands for the input parameters, however, isn't going to >> > work around the fact that _this_ assembly is written to be out of >> > line and so it must never be inlined by the compiler. >> >> I briefly looked at a disassembled version after applying both patches, >> it indeed leads to inlining. However, the code seems to be working >> (thanks to asm volatile?)... > > Apart from v4wb_copy_user_page() and mc_copy_user_page(), how is > Clang inlining these static functions that are only used through > function pointers?
I only looked at copypage-xscale.c (the mc_copy_user_page() case)... -- Stefan