Messages in this thread | | | Date | Mon, 15 Oct 2018 23:46:15 +0100 | From | Russell King - ARM Linux <> | Subject | Re: [PATCH 1/2] ARM: copypage-fa: add kto and kfrom to input operands list |
| |
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?
-- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up
| |