Re: [PATCH v3 1/2] perf util: Check for fused instruction

From: Jin, Yao
Date: Thu Jul 06 2017 - 21:10:09 EST

+static bool x86__ins_is_fused(char *cpuid, const char *ins1, const char *ins2)
+ unsigned int family, model, stepping;
+ int ret;
+ /*
+ * cpuid = "GenuineIntel,family,model,stepping"
+ */
+ ret = sscanf(cpuid, "%*[^,],%u,%u,%u", &family, &model, &stepping);
So, looking at the next patch, that uses this, I see that you'll call
this everytime that jump arrow will be printed, why not do this when
doing the initial disassembly, and having this info cached in the struct
ins or disasm_line (haven't looked closely to provide exact instructions
on how to do it)?

Yes, you're right. It doesn't need to do this model checking each time.

I will move the model checking to other place and record the model in annotate_browser.

Even more, you could do this model checking just once per disassembly,
then use it as you go reading the disassembly lines, marking them as
fused/not fused and then at jump arror printing just look at a flag, no?

- Arnaldo

For this, I have another consideration.

The fused instruction pair consists of 2 instructions. One is jump, the other is "CMP/TEST/...".

If I check the fused instruction pair once per disassembly, I need to check if one instruction is jump and the other is "CMP/TEST/..." for all instructions.

While in annotate_browser__draw_current_jump(), it has already checked if a disasm_line is a valid jump, so the jump checking will be duplicated.

The current implementation is just performing the fused checking when user moves the cursor on the jump instruction. It doesn't need to add additional jump checking.

So the current way may be acceptable as well?

Jin Yao