On Thu, Jul 09, 2020 at 09:33:18AM -0400, Mathieu Desnoyers wrote: > > The way this all uses r17 will likely not work reliably. > > r17 is only used as a temporary register within the inline assembler, and it > is > in the clobber list. In which scenario would it not work reliably ?
This isn't clear at all, that is the problem. > > The way multiple asm statements are used seems to have missing > > dependencies between the statements. > > I'm not sure I follow here. Note that we are injecting the CPP macros into > a single inline asm statement as strings. Yeah... more trickiness. > > And done macro-mess this, you want to be able to debug it, and you need > > other people to be able to read it! > > I understand that looking at macros can be cumbersome from the perspective > of a reviewer only interested in a single architecture, No, from the perspective of *any* reviewer. > However, from my perspective, as a maintainer who must maintain similar code > for x86 32/64, powerpc 32/64, arm, aarch64, s390, s390x, mips 32/64, and > likely > other architectures in the future, the macros abstracting 32-bit and 64-bit > allow to eliminate code duplication for each architecture with 32-bit and > 64-bit > variants, which is better for maintainability. IMNSHO it is MUCH better to just have simple separate implementations for each. They differ in *all* details. Or have static inline functions, with proper dependencies, instead of nasty text macros. But it's your code, do what you want :-) Segher
