-
Notifications
You must be signed in to change notification settings - Fork 252
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
disable "Dissasembly" function for inlined functions (size calculation of inline functions not possible) #548
Comments
I fear without an MWE we can't really invesetigate this. You'll have to debug this yourself |
I try, can you please update HACKING on how to correctly set this beast up for debugging? I've manually adjusted a bunch of cmake files to get "something" reasonable, but this must be easier... [Also needed: a note about how to run the tests and add new ones.] |
what do you mean exactly, i.e. what's your problem? Once you can compile it you can debug it, no? Adjusting cmake files cannot be correct, but without knowing what you are trying to do I cannot give you any hint on what the right approach would but. This is just a totally normal cmake project after all, there's zero special about hotspot in any regard. Are you maybe struggling with cmake in general? Just set |
While this is just a example it possibly shows the problem: #include <stdlib.h>
#include <string.h>
static void __inline __attribute__((always_inline)) do_check (char *c)
{
if (!memcmp(c, "abcd", 4)
|| !memcmp(c, "ABCD", 4)
|| !memcmp(c, "DCBA", 4)
|| !memcmp(c, "dcba", 4)) {
exit (1);
}
}
#define code(pos) \
for (int i = 0; i < 99999; i++) { \
checkme[pos] = (char)i; \
do_check(checkme); \
}
#define codes code(1) code (2) code (3) code (0)
#define codes2 codes codes codes codes codes codes codes
#define codes3 codes2 codes2 codes2 codes2 codes2 codes2
#define codes4 codes3 codes3 codes3 codes3 codes3 codes3
int main () {
char checkme[4] = { 99, 98, 97, 96};
for (int i = 0; i < 9999999; i++) {
checkme[3] = (char)i;
do_check(checkme);
}
codes4
do_check(checkme);
return 0;
} $> gcc mega.c -ggdb3
$> perf record --call-graph dwarf,28192 --aio -z --mmap-pages 16M ./a.out
$> hotspot Then use disassembly on |
Note: I've also not recognized any cycles in the disassembly (but more or less the expected output in the cycles within the source view). |
@milianw Can you reproduce it (= is that enough of a minimal reproducible example)? |
I can reproduce it, but is this really a bug? The code is inlined after all, and thus basically the whole app is part of do_check, no? We could disable the disassembly support for inlined functions, which is I believe something that @lievenhey suggested to do anyways. But I don't see any alternative to this case here - what do you have in mind @GitMensch ? |
As most commonly the inlined functions will result in the same generated assembler, I suggest to:
|
Your assumption is completely wrong. Once a function gets inlined, all bets are off - there are most definitely no guarantees that the produced assembly is the same. actually, quite the contrary, the idea is to let the context of the parent function allow for further optimizations that would drastically change the inlined code. furthermore, the compiler cannot give us a range for an inlined function, as it doesn't have any clear boundaries anymore. what you are requesting is simply completely at odds with reality |
If we can't get the range of a single inline, then how would @lievenhey be able to collapse them? As noted already in the issue starter, the idea is
Note that this "only" applies to the inline function in a single function (in the sample down as main::do_check), not to other inline versions as sub::do_check. If this really isn't possible, then we may should disable the disassembly of the inline function itself, the GUI could ask "Not possible, do you want to disassemble the containing function 'main' instead?" |
I use the debug info annotations. They have references to the source code (and in case of inlined functions they mark the function as inlined). But that doesn't really help in your case, since the compiler can also reorder code which can spread out the inlined function like this:
As @milianw said, inlined functions throw off all bets, since the compiler can also remove parts of the inlined function if he can assure that these paths would never be executed. inline int foo(bool b) {
if (b) return 2;
return 4;
}
int bar() {
return foo(false) + foo(true);
} Under the assumption that bar:
mov %rax, 4
mov %rbx, 2
add %rax, %rax, %rbx
ret As you can see the branch was completely eliminated by the compiler. |
And don't forget that inlined functions can call other inlined functions, which will get even more messy |
I've adjusted the issue title. As you both said: showing the disassembly of inlined functions is only possible within another function. Therefore: please disable that, possible hinting to the user that he may disassemble a function where this inlined function is included. |
It is not possible to disassemble a inlined functions. This patch disables the disassembly entry in the context menu for these functions. fixes: #548
It is not possible to disassemble a inlined functions. This patch disables the disassembly entry in the context menu for these functions. fixes: #548
It is not possible to disassemble a inlined functions. This patch disables the disassembly entry in the context menu for these functions. fixes: #548
It is not possible to disassemble a inlined functions. This patch disables the disassembly entry in the context menu for these functions. fixes: #548
It is not possible to disassemble a inlined functions. This patch disables the disassembly entry in the context menu for these functions. fixes: #548
It is not possible to disassemble a inlined functions. This patch disables the disassembly entry in the context menu for these functions. fixes: #548
It is not possible to disassemble a inlined functions. This patch disables the disassembly entry in the context menu for these functions. fixes: #548
It is not possible to disassemble a inlined functions. This patch disables the disassembly entry in the context menu for these functions. fixes: #548
It is not possible to disassemble a inlined functions. This patch disables the disassembly entry in the context menu for these functions. fixes: #548
It is not possible to disassemble a inlined functions. This patch disables the disassembly entry in the context menu for these functions. fixes: #548
It is not possible to disassemble a inlined functions. This patch disables the disassembly entry in the context menu for these functions. fixes: #548
It is not possible to disassemble a inlined functions. This patch disables the disassembly entry in the context menu for these functions. fixes: #548
It is not possible to disassemble a inlined functions. This patch disables the disassembly entry in the context menu for these functions. fixes: #548
It is not possible to disassemble a inlined functions. This patch disables the disassembly entry in the context menu for these functions. fixes: #548
Describe the bug
The size is definitely wrong (the inlined code consists of two 8 byte memcmp which are both done and a function call that is never executed). This leads to not usable disassembly view:
To Reproduce
Steps to reproduce the behavior:
Expected behavior
Open to discussion; the actual disassembly for inlined functions possibly could vary (I'm not sure), otherwise either disassemble the inlined version that has most cost (if we know that), otherwise the first one
The text was updated successfully, but these errors were encountered: