On Tue, Mar 27, 2012 at 11:33 PM, Richard Henderson <r...@twiddle.net> wrote: > On 03/27/12 02:24, Jia Liu wrote: >> +ifeq ($(TARGET_BASE_ARCH), mips) >> +libobj-y += dsp_helper.o >> +endif > ... > >> +#include "dyngen-exec.h" >> + >> +/*** MIPS DSP internal functions begin ***/ >> +static inline void set_DSPControl_overflow_flag(uint32_t flag, int position) >> +{ >> + env->active_tc.DSPControl |= (target_ulong)flag << position; >> +} > > In order to use dyngen-exec.h, and the global ENV variable, > you must also do > > dsp_helper.o: QEMU_CFLAGS += $(HELPER_CFLAGS) > > in the Makefile.target. If you had tested i386 host instead > of x86_64, you should have seen this failure. > > Better, from the point of view of one working toward eliminating > dyngen-exec.h and these special calling conventions, is to actually > pass down env in the helper. E.g. >
Thanks, I'll add it. > +/** DSP Arithmetic Sub-class insns **/ > +uint32_t helper_addq_ph(CPUMIPSState *env, uint32_t rs, uint32_t rt) > > +DEF_HELPER_FLAGS_3(addq_ph, 0, i32, env, i32, i32) > > where mipsdsp_add_i16 and thence set_DSPControl_overflow_flag also > grow ENV parameters. > > In addition, we can't use TCG_CALL_CONST | TCG_CALL_PURE on any > function that sets DSPControl, because that's the cpu_dspctrl tcg > variable. > > You might get better performance out of TCG if you do *not* make > DSPControl a TCG variable, but instead perform explicit loads from > that slot in the few places that need it. That way you can at > least set TCG_CALL_CONST. (Though not TCG_CALL_PURE, because the > functions do have side effects.) > Thanks. do you mean, I should write like this? helper.h: DEF_HELPER_FLAGS_3(addq_ph, 0, i32, env, i32, i32) dsp_helper.c: uint32_t helper_addq_ph(CPUMIPSState *env, uint32_t rs, uint32_t rt) {} Sorry I think I'm not turly understand your comment. >> + if (len == 2) >> + env->active_tc.DSPControl &= 0xFCFFFFFF; >> + else if (len == 4) >> + env->active_tc.DSPControl &= 0xF0FFFFFF; > > Run all your patches through ./scripts/checkpatch.pl and fix the > errors that will report. > I've run ./scripts/checkpatch.pl, but I didn't get a ERR here... It do have some ERR, but the all the other Macros write like that, so I didn't have a idea about that. > > r~ Regards, Jia.