| Message ID | 20230610171959.928544-1-richard.henderson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | tests/plugin: Remove duplicate insn log from libinsn.so | expand |
Richard Henderson <richard.henderson@linaro.org> writes: > This is a perfectly natural occurrence for x86 "rep movb", > where the "rep" prefix forms a counted loop of the one insn. > > During the tests/tcg/multiarch/memory test, this logging is > triggered over 350000 times. Within the context of cross-i386-tci > build, which is already slow by nature, the logging is sufficient > to push the test into timeout. How does this get triggered because I added these: # non-inline runs will trigger the duplicate instruction heuristics in libinsn.so run-plugin-%-with-libinsn.so: $(call run-test, $@, \ $(QEMU) -monitor none -display none \ -chardev file$(COMMA)path=$@.out$(COMMA)id=output \ -plugin ../../plugin/libinsn.so$(COMMA)inline=on \ -d plugin -D $*-with-libinsn.so.pout \ $(QEMU_OPTS) $*) to prevent the callback versions from being called for x86. The original intent of the check was to detect failures due to cpu_io_recompile, see e025d799af (tests/plugin: expand insn test to detect duplicate instructions) > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > Irritatingly, it doesn't timeout locally, so I used staging to double-check: > > Fail: https://gitlab.com/qemu-project/qemu/-/jobs/4450754282#L5062 > Pass: https://gitlab.com/qemu-project/qemu/-/jobs/4450927108 > --- > tests/plugin/insn.c | 9 +-------- > 1 file changed, 1 insertion(+), 8 deletions(-) > > diff --git a/tests/plugin/insn.c b/tests/plugin/insn.c > index cd5ea5d4ae..9bd6e44f73 100644 > --- a/tests/plugin/insn.c > +++ b/tests/plugin/insn.c > @@ -19,7 +19,6 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION; > #define MAX_CPUS 8 /* lets not go nuts */ > > typedef struct { > - uint64_t last_pc; > uint64_t insn_count; > } InstructionCount; > > @@ -51,13 +50,7 @@ static void vcpu_insn_exec_before(unsigned int cpu_index, void *udata) > { > unsigned int i = cpu_index % MAX_CPUS; > InstructionCount *c = &counts[i]; > - uint64_t this_pc = GPOINTER_TO_UINT(udata); > - if (this_pc == c->last_pc) { > - g_autofree gchar *out = g_strdup_printf("detected repeat execution @ 0x%" > - PRIx64 "\n", this_pc); > - qemu_plugin_outs(out); > - } > - c->last_pc = this_pc; > + > c->insn_count++; > }
On 6/11/23 02:14, Alex Bennée wrote: > > Richard Henderson <richard.henderson@linaro.org> writes: > >> This is a perfectly natural occurrence for x86 "rep movb", >> where the "rep" prefix forms a counted loop of the one insn. >> >> During the tests/tcg/multiarch/memory test, this logging is >> triggered over 350000 times. Within the context of cross-i386-tci >> build, which is already slow by nature, the logging is sufficient >> to push the test into timeout. > > How does this get triggered because I added these: > > # non-inline runs will trigger the duplicate instruction heuristics in libinsn.so > run-plugin-%-with-libinsn.so: > $(call run-test, $@, \ > $(QEMU) -monitor none -display none \ > -chardev file$(COMMA)path=$@.out$(COMMA)id=output \ > -plugin ../../plugin/libinsn.so$(COMMA)inline=on \ > -d plugin -D $*-with-libinsn.so.pout \ > $(QEMU_OPTS) $*) > > to prevent the callback versions from being called for x86. The original > intent of the check was to detect failures due to cpu_io_recompile, see > e025d799af (tests/plugin: expand insn test to detect duplicate instructions) I have no idea how, but it's happening. >> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> >> --- >> Irritatingly, it doesn't timeout locally, so I used staging to double-check: >> >> Fail: https://gitlab.com/qemu-project/qemu/-/jobs/4450754282#L5062 >> Pass: https://gitlab.com/qemu-project/qemu/-/jobs/4450927108 Note that in the pass case, we don't even log that the test ran. r~
On 6/12/23 04:50, Richard Henderson wrote: > On 6/11/23 02:14, Alex Bennée wrote: >> >> Richard Henderson <richard.henderson@linaro.org> writes: >> >>> This is a perfectly natural occurrence for x86 "rep movb", >>> where the "rep" prefix forms a counted loop of the one insn. >>> >>> During the tests/tcg/multiarch/memory test, this logging is >>> triggered over 350000 times. Within the context of cross-i386-tci >>> build, which is already slow by nature, the logging is sufficient >>> to push the test into timeout. >> >> How does this get triggered because I added these: >> >> # non-inline runs will trigger the duplicate instruction heuristics in libinsn.so >> run-plugin-%-with-libinsn.so: >> $(call run-test, $@, \ >> $(QEMU) -monitor none -display none \ >> -chardev file$(COMMA)path=$@.out$(COMMA)id=output \ >> -plugin ../../plugin/libinsn.so$(COMMA)inline=on \ >> -d plugin -D $*-with-libinsn.so.pout \ >> $(QEMU_OPTS) $*) >> >> to prevent the callback versions from being called for x86. The original >> intent of the check was to detect failures due to cpu_io_recompile, see >> e025d799af (tests/plugin: expand insn test to detect duplicate instructions) > > I have no idea how, but it's happening. > > >>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> >>> --- >>> Irritatingly, it doesn't timeout locally, so I used staging to double-check: >>> >>> Fail: https://gitlab.com/qemu-project/qemu/-/jobs/4450754282#L5062 >>> Pass: https://gitlab.com/qemu-project/qemu/-/jobs/4450927108 > > Note that in the pass case, we don't even log that the test ran. Any further thoughts on this? Otherwise I'll merge it to get rid of the cross-i386-tci failure... r~
Richard Henderson <richard.henderson@linaro.org> writes: > On 6/12/23 04:50, Richard Henderson wrote: >> On 6/11/23 02:14, Alex Bennée wrote: >>> >>> Richard Henderson <richard.henderson@linaro.org> writes: >>> >>>> This is a perfectly natural occurrence for x86 "rep movb", >>>> where the "rep" prefix forms a counted loop of the one insn. >>>> >>>> During the tests/tcg/multiarch/memory test, this logging is >>>> triggered over 350000 times. Within the context of cross-i386-tci >>>> build, which is already slow by nature, the logging is sufficient >>>> to push the test into timeout. >>> >>> How does this get triggered because I added these: >>> >>> # non-inline runs will trigger the duplicate instruction heuristics in libinsn.so >>> run-plugin-%-with-libinsn.so: >>> $(call run-test, $@, \ >>> $(QEMU) -monitor none -display none \ >>> -chardev file$(COMMA)path=$@.out$(COMMA)id=output \ >>> -plugin ../../plugin/libinsn.so$(COMMA)inline=on \ >>> -d plugin -D $*-with-libinsn.so.pout \ >>> $(QEMU_OPTS) $*) >>> >>> to prevent the callback versions from being called for x86. The original >>> intent of the check was to detect failures due to cpu_io_recompile, see >>> e025d799af (tests/plugin: expand insn test to detect duplicate instructions) >> I have no idea how, but it's happening. >> >>>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> >>>> --- >>>> Irritatingly, it doesn't timeout locally, so I used staging to double-check: >>>> >>>> Fail: https://gitlab.com/qemu-project/qemu/-/jobs/4450754282#L5062 >>>> Pass: https://gitlab.com/qemu-project/qemu/-/jobs/4450927108 >> Note that in the pass case, we don't even log that the test ran. > > Any further thoughts on this? Otherwise I'll merge it to get rid of > the cross-i386-tci failure... > > > r~ I'm happy to drop the feature from the plugin but the clean-up also needs to be applied to the run-plugin-%-with-libinsn.so: rules for i386 and x86_64.
On 6/19/23 19:34, Alex Bennée wrote: > > Richard Henderson <richard.henderson@linaro.org> writes: > >> On 6/12/23 04:50, Richard Henderson wrote: >>> On 6/11/23 02:14, Alex Bennée wrote: >>>> >>>> Richard Henderson <richard.henderson@linaro.org> writes: >>>> >>>>> This is a perfectly natural occurrence for x86 "rep movb", >>>>> where the "rep" prefix forms a counted loop of the one insn. >>>>> >>>>> During the tests/tcg/multiarch/memory test, this logging is >>>>> triggered over 350000 times. Within the context of cross-i386-tci >>>>> build, which is already slow by nature, the logging is sufficient >>>>> to push the test into timeout. >>>> >>>> How does this get triggered because I added these: >>>> >>>> # non-inline runs will trigger the duplicate instruction heuristics in libinsn.so >>>> run-plugin-%-with-libinsn.so: >>>> $(call run-test, $@, \ >>>> $(QEMU) -monitor none -display none \ >>>> -chardev file$(COMMA)path=$@.out$(COMMA)id=output \ >>>> -plugin ../../plugin/libinsn.so$(COMMA)inline=on \ >>>> -d plugin -D $*-with-libinsn.so.pout \ >>>> $(QEMU_OPTS) $*) >>>> >>>> to prevent the callback versions from being called for x86. The original >>>> intent of the check was to detect failures due to cpu_io_recompile, see >>>> e025d799af (tests/plugin: expand insn test to detect duplicate instructions) >>> I have no idea how, but it's happening. >>> >>>>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> >>>>> --- >>>>> Irritatingly, it doesn't timeout locally, so I used staging to double-check: >>>>> >>>>> Fail: https://gitlab.com/qemu-project/qemu/-/jobs/4450754282#L5062 >>>>> Pass: https://gitlab.com/qemu-project/qemu/-/jobs/4450927108 >>> Note that in the pass case, we don't even log that the test ran. >> >> Any further thoughts on this? Otherwise I'll merge it to get rid of >> the cross-i386-tci failure... >> >> >> r~ > > I'm happy to drop the feature from the plugin but the clean-up also > needs to be applied to the run-plugin-%-with-libinsn.so: rules for i386 > and x86_64. Pardon? I don't know what you mean wrt changing the makefile. r~
Richard Henderson <richard.henderson@linaro.org> writes: > On 6/19/23 19:34, Alex Bennée wrote: >> Richard Henderson <richard.henderson@linaro.org> writes: >> >>> On 6/12/23 04:50, Richard Henderson wrote: >>>> On 6/11/23 02:14, Alex Bennée wrote: >>>>> >>>>> Richard Henderson <richard.henderson@linaro.org> writes: >>>>> >>>>>> This is a perfectly natural occurrence for x86 "rep movb", >>>>>> where the "rep" prefix forms a counted loop of the one insn. >>>>>> >>>>>> During the tests/tcg/multiarch/memory test, this logging is >>>>>> triggered over 350000 times. Within the context of cross-i386-tci >>>>>> build, which is already slow by nature, the logging is sufficient >>>>>> to push the test into timeout. >>>>> >>>>> How does this get triggered because I added these: >>>>> >>>>> # non-inline runs will trigger the duplicate instruction heuristics in libinsn.so >>>>> run-plugin-%-with-libinsn.so: >>>>> $(call run-test, $@, \ >>>>> $(QEMU) -monitor none -display none \ >>>>> -chardev file$(COMMA)path=$@.out$(COMMA)id=output \ >>>>> -plugin ../../plugin/libinsn.so$(COMMA)inline=on \ >>>>> -d plugin -D $*-with-libinsn.so.pout \ >>>>> $(QEMU_OPTS) $*) >>>>> >>>>> to prevent the callback versions from being called for x86. The original >>>>> intent of the check was to detect failures due to cpu_io_recompile, see >>>>> e025d799af (tests/plugin: expand insn test to detect duplicate instructions) >>>> I have no idea how, but it's happening. >>>> >>>>>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> >>>>>> --- >>>>>> Irritatingly, it doesn't timeout locally, so I used staging to double-check: >>>>>> >>>>>> Fail: https://gitlab.com/qemu-project/qemu/-/jobs/4450754282#L5062 >>>>>> Pass: https://gitlab.com/qemu-project/qemu/-/jobs/4450927108 >>>> Note that in the pass case, we don't even log that the test ran. >>> >>> Any further thoughts on this? Otherwise I'll merge it to get rid of >>> the cross-i386-tci failure... >>> >>> >>> r~ >> I'm happy to drop the feature from the plugin but the clean-up also >> needs to be applied to the run-plugin-%-with-libinsn.so: rules for i386 >> and x86_64. > > Pardon? I don't know what you mean wrt changing the makefile. There are a couple of places that do: # non-inline runs will trigger the duplicate instruction heuristics in libinsn.so run-plugin-%-with-libinsn.so: $(call run-test, $@, $(QEMU) $(QEMU_OPTS) \ -plugin ../../plugin/libinsn.so$(COMMA)inline=on \ -d plugin -D $*-with-libinsn.so.pout $*) to prevent triggering the assert for x86
diff --git a/tests/plugin/insn.c b/tests/plugin/insn.c index cd5ea5d4ae..9bd6e44f73 100644 --- a/tests/plugin/insn.c +++ b/tests/plugin/insn.c @@ -19,7 +19,6 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION; #define MAX_CPUS 8 /* lets not go nuts */ typedef struct { - uint64_t last_pc; uint64_t insn_count; } InstructionCount; @@ -51,13 +50,7 @@ static void vcpu_insn_exec_before(unsigned int cpu_index, void *udata) { unsigned int i = cpu_index % MAX_CPUS; InstructionCount *c = &counts[i]; - uint64_t this_pc = GPOINTER_TO_UINT(udata); - if (this_pc == c->last_pc) { - g_autofree gchar *out = g_strdup_printf("detected repeat execution @ 0x%" - PRIx64 "\n", this_pc); - qemu_plugin_outs(out); - } - c->last_pc = this_pc; + c->insn_count++; }
This is a perfectly natural occurrence for x86 "rep movb", where the "rep" prefix forms a counted loop of the one insn. During the tests/tcg/multiarch/memory test, this logging is triggered over 350000 times. Within the context of cross-i386-tci build, which is already slow by nature, the logging is sufficient to push the test into timeout. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- Irritatingly, it doesn't timeout locally, so I used staging to double-check: Fail: https://gitlab.com/qemu-project/qemu/-/jobs/4450754282#L5062 Pass: https://gitlab.com/qemu-project/qemu/-/jobs/4450927108 --- tests/plugin/insn.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-)