Re: [PATCH] Do not inline virNumaNodeIsAvailable

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Mar 09, 2015 at 10:09:17AM +0000, Daniel P. Berrange wrote:
On Mon, Mar 09, 2015 at 11:01:31AM +0100, Martin Kletzander wrote:
On Thu, Mar 05, 2015 at 11:09:41AM +0000, Daniel P. Berrange wrote:
>On Thu, Mar 05, 2015 at 12:05:52PM +0100, Ján Tomko wrote:
>>Explicitly request that virNumaNodeIsAvailable not be inlined.
>>This fixes the test suite when building with clang (3.5.1).
>
>Huh, so clang will inline functions, even if they are exported
>in the .so library ?  Is there some clang compiler flag we can
>use to stop that ?  I'd only expect it to inline stuff which
>was declared static, or whose impl body was in the header file
>

If I understand it correctly, that means that clang is not
"compatible" with gcc.

Excerpt from gcc online docs [1]:

 When a function is both inline and static, if all calls to the
 function are integrated into the caller, and the function's address
 is never used, then the function's own assembler code is never
 referenced.

Excerpt from gcc online docs [1]:

 By default, Clang builds C code in GNU C11 mode, so it uses standard
 C99 semantics for the inline keyword. These semantics are different
 from those in GNU C89 mode, which is the default mode in versions of
 GCC prior to 5.0.

However further reading of the second documentation and c89 semantics
it doesn't say anything about the fact that such function should be
inlined.

But we haven't added the 'inline' keyword to this function at
all - it is just a normal function marked for export in the
.so file, so I'm puzzelled why it is getting inlined.


Exactly, that's what I'm trying to find out as well.


Anyway, is this clang 3.6 specific?  I don't have this problem when
compiling with 3.5.  Nor does this show with gcc -std=gnu11.  I'm
getting 3.6 to check whether that's the difference.


After updating clang and llvm from 3.5 to 3.6, I still don't get this
error.  And I have only 4 (fake) nodes available, so it _is_ rewriting
that function.

[1] https://gcc.gnu.org/onlinedocs/gcc/Inline.html
[2] http://clang.llvm.org/compatibility.html

>>---
>>This only leaves the mysterious check-protocol failure.

That's not that mysterious, it's just that we check the order and
clang sorts enums before structs, but gcc doesn't.  Also clang adds
"public:" to structs, so it probably treats it as a C++ or C# structs
or something?


By the way if I compile with clang with -std=gnu11 or -std=gnu99, the
"public:" stuff is gone :)

>>And too large stack frame size when building the tests with -O0.
>>

I'm using CFLAGS='-O0 -ggdb -Wno-frame-larger-than=' for this as a
parameter for autogen.sh O:-)

>> src/internal.h     | 10 ++++++++++
>> src/util/virnuma.h |  3 ++-
>> 2 files changed, 12 insertions(+), 1 deletion(-)
>>
>>diff --git a/src/internal.h b/src/internal.h
>>index 4d473af..84aa330 100644
>>--- a/src/internal.h
>>+++ b/src/internal.h
>>@@ -139,6 +139,16 @@
>> #  endif
>>
>> /**
>>+ * ATTRIBUTE_NOINLINE:
>>+ *
>>+ * Macro to indicate a function that cannot be inlined
>>+ * (e.g. a function that is mocked in the test suite)
>>+ */
>>+#  ifndef ATTRIBUTE_NOINLINE
>>+#   define ATTRIBUTE_NOINLINE __attribute__((__noinline__))
>>+#  endif
>>+
>>+/**
>>  * ATTRIBUTE_SENTINEL:
>>  *
>>  * Macro to check for NULL-terminated varargs lists
>>diff --git a/src/util/virnuma.h b/src/util/virnuma.h
>>index 1f3c0ad..4ddcc5a 100644
>>--- a/src/util/virnuma.h
>>+++ b/src/util/virnuma.h
>>@@ -37,7 +37,8 @@ virBitmapPtr virNumaGetHostNodeset(void);
>> bool virNumaNodesetIsAvailable(virBitmapPtr nodeset);
>> bool virNumaIsAvailable(void);
>> int virNumaGetMaxNode(void);
>>-bool virNumaNodeIsAvailable(int node);
>>+bool virNumaNodeIsAvailable(int node)
>>+    ATTRIBUTE_NOINLINE;
>> int virNumaGetDistances(int node,
>>                         int **distances,
>>                         int *ndistances);
>
>Regards,
>Daniel
>--
>|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
>|: http://libvirt.org              -o-             http://virt-manager.org :|
>|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
>|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|
>
>--
>libvir-list mailing list
>libvir-list@xxxxxxxxxx
>https://www.redhat.com/mailman/listinfo/libvir-list



Regards,
Daniel
--
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Attachment: pgpLKYIf4uQrJ.pgp
Description: PGP signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]