Hi Jan,

On 19/11/2020 10:27, Jan Beulich wrote:
On 18.11.2020 19:09, Julien Grall wrote:
On 23/10/2020 11:19, Jan Beulich wrote:
--- a/xen/include/xen/compiler.h
+++ b/xen/include/xen/compiler.h
@@ -12,6 +12,7 @@
#define inline __inline__
   #define always_inline __inline__ __attribute__ ((__always_inline__))
+#define gnu_inline    __inline__ __attribute__ ((__gnu_inline__))

bsearch() is only used by Arm and I haven't seen anyone so far
complaining about the perf of I/O emulation.

Therefore, I am not convinced that there is enough justification to
introduce a GNU attribute just for this patch.

Please settle this with Andrew: He had asked for the function to
become inline. I don't view making it static inline in the header
as an option here - if the compiler decides to not inline it, we
should not end up with multiple instances in different CUs.

That's the cons of static inline... but then why is it suddenly a problem with this helper?

And
without making it static inline the attribute needs adding; at
least I'm unaware of an alternative which works with the various
compiler versions.

The question we have to answer is: What is the gain with this approach?

If it is not quantifiable, then introducing compiler specific attribute is not an option.

IIRC, there are only two callers (all in Arm code) of this function. Even inlined, I don't believe you would drastically reduce the number of instructions compare to a full blown version. To be generous, I would say you may save ~20 instructions per copy.

Therefore, so far, the compiler specific attribute doesn't look justified to me. As usual, I am happy to be proven wrong.

Cheers,

--
Julien Grall

Reply via email to