[llvm-dev] XRay threshold (bug?)

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

[llvm-dev] XRay threshold (bug?)

Gerolf Hoflehner via llvm-dev
Hi Dean,

I've noticed that XRay in llvm\lib\CodeGen\XRayInstrumentation.cpp compares its threshold against Function::size() . However, Function::size() returns the number of basic blocks (as I understand, such as cycle bodies, if/else bodies, switch-case bodies, etc.), rather than the number of instructions.

Was your intent to count the instructions instead? The name of the parameter -fxray-instruction-threshold=N suggests this, as well as XRay documentation at http://llvm.org/docs/XRay.html .
If so, I see two options:
1. Count the number of MachineInstr`s in MachineFunction : this gives better  estimate for the number of assembly instructions on the target. So a user can check in disassembly that the threshold works more or less correctly.
2. Count the number of Instruction`s in a Function : AFAIK, this gives correct number of IR instructions, which the user can check in IR listing. However, this number may be far (several times for small functions) from the number of assembly instructions finally emitted.

My team is in favor of option 1 (proposed patch here https://reviews.llvm.org/D34027 ), because we think that having the closer estimate for the number of assembly instructions emitted is more important than to have a clear definition of the metric.

What do you think?

Cheers,
Serge

_______________________________________________
LLVM Developers mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
Reply | Threaded
Open this post in threaded view
|

Re: [llvm-dev] XRay threshold (bug?)

Gerolf Hoflehner via llvm-dev
Hi Serge,

Yes I like option 1 as well, if size() isn't actually the machine instruction count. My only concern might be the cost of counting the instructions, but if it's only when XRay instrumentation is on then this should be fine.

Let me have a look at the patch.

Cheers

Sent from my iPhone

On 8 Jun 2017, at 9:26 pm, Serge Rogatch via llvm-dev <[hidden email]> wrote:

Hi Dean,

I've noticed that XRay in llvm\lib\CodeGen\XRayInstrumentation.cpp compares its threshold against Function::size() . However, Function::size() returns the number of basic blocks (as I understand, such as cycle bodies, if/else bodies, switch-case bodies, etc.), rather than the number of instructions.

Was your intent to count the instructions instead? The name of the parameter -fxray-instruction-threshold=N suggests this, as well as XRay documentation at http://llvm.org/docs/XRay.html .
If so, I see two options:
1. Count the number of MachineInstr`s in MachineFunction : this gives better  estimate for the number of assembly instructions on the target. So a user can check in disassembly that the threshold works more or less correctly.
2. Count the number of Instruction`s in a Function : AFAIK, this gives correct number of IR instructions, which the user can check in IR listing. However, this number may be far (several times for small functions) from the number of assembly instructions finally emitted.

My team is in favor of option 1 (proposed patch here https://reviews.llvm.org/D34027 ), because we think that having the closer estimate for the number of assembly instructions emitted is more important than to have a clear definition of the metric.

What do you think?

Cheers,
Serge
_______________________________________________
LLVM Developers mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

_______________________________________________
LLVM Developers mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
Reply | Threaded
Open this post in threaded view
|

Re: [llvm-dev] XRay threshold (bug?)

Gerolf Hoflehner via llvm-dev
The old version was counting the number of basic blocks by iterating through them, because the list doesn't keep the count of items in it. So the complexity was linear in the number of basic blocks.
The new version counts the number of instructions also by iterating them. So the complexity is linear in the number of instructions.
This effectively adds another pass over the whole program code, more specifically, over all the MachineInstr`s, because the above iteration is performed for each function. However, this pass is as light as possible because it doesn't do anything for each instruction except counting it.
I believe this pass is performed only when `-fxray-instrument` option is specified, because XRayInstrumentation::runOnMachineFunction() is only run in that case.

Cheers,
Serge

On 8 June 2017 at 14:41, Dean Michael Berris <[hidden email]> wrote:
Hi Serge,

Yes I like option 1 as well, if size() isn't actually the machine instruction count. My only concern might be the cost of counting the instructions, but if it's only when XRay instrumentation is on then this should be fine.

Let me have a look at the patch.

Cheers

Sent from my iPhone

On 8 Jun 2017, at 9:26 pm, Serge Rogatch via llvm-dev <[hidden email]> wrote:

Hi Dean,

I've noticed that XRay in llvm\lib\CodeGen\XRayInstrumentation.cpp compares its threshold against Function::size() . However, Function::size() returns the number of basic blocks (as I understand, such as cycle bodies, if/else bodies, switch-case bodies, etc.), rather than the number of instructions.

Was your intent to count the instructions instead? The name of the parameter -fxray-instruction-threshold=N suggests this, as well as XRay documentation at http://llvm.org/docs/XRay.html .
If so, I see two options:
1. Count the number of MachineInstr`s in MachineFunction : this gives better  estimate for the number of assembly instructions on the target. So a user can check in disassembly that the threshold works more or less correctly.
2. Count the number of Instruction`s in a Function : AFAIK, this gives correct number of IR instructions, which the user can check in IR listing. However, this number may be far (several times for small functions) from the number of assembly instructions finally emitted.

My team is in favor of option 1 (proposed patch here https://reviews.llvm.org/D34027 ), because we think that having the closer estimate for the number of assembly instructions emitted is more important than to have a clear definition of the metric.

What do you think?

Cheers,
Serge
_______________________________________________
LLVM Developers mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev


_______________________________________________
LLVM Developers mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev