On 5/4/20 11:30 AM, BALATON Zoltan wrote:
> On Mon, 4 May 2020, Richard Henderson wrote:
>> On 5/3/20 5:41 PM, 罗勇刚(Yonggang Luo) wrote:
>>> On Mon, May 4, 2020 at 7:40 AM BALATON Zoltan <bala...@eik.bme.hu
>>> <mailto:bala...@eik.bme.hu>> wrote:
>>>
>>> Hello,
>>>
>>> On Mon, 4 May 2020, 罗勇刚(Yonggang Luo) wrote:
>>> > Hello Richard, Can you have a look at the following patch, and was that
>>> are
>>> > the right direction?
>>>
>>> Formatting of the patch is broken by your mailer, try sending it with
>>> something that does not change it otherwise it's a bit hard to read.
>>>
>>> Richard suggested to add an assert to check the fp_status is correctly
>>> cleared in place of helper_reset_fpstatus first for debugging so you
>>> could
>>> change the helper accordingly before deleting it and run a few tests to
>>> verify it still works. You'll need get some tests and benchmarks working
>>> to be able to verify your changes that's why I've said that would be
>>> step
>>> 0. If you checked that it still produces the same results and the assert
>>> does not trigger then you can remove the helper.
>>>
>>> That's what I need help,
>>> 1. How to write a assert to replace helper_reset_fpstatus .
>>> just directly assert? or something else
>>
>> You can't place the assert where helper_reset_fpstatus was. You need to
>> place
>> it in each of the helpers, like helper_fadd, that previously has a call to
>> helper_reset_fpstatus preceeding it.
>
> Why? If we want to verify that clearing fp_status after flags are processed is
> equivalent to clearing flags before fp ops then verifying that the fp_status
> is
> already cleared when the current helper_reset_fpstatus is called should be
> enough to check that nothing has set the flags in between so the current reset
> helper would be no op. Therefore I thought you could put the assert there for
> checking this. This assert is for debugging and checking the change only and
> not meant to be left there otherwise we lose all the performance gain so it's
> easier to put in the current helper before removing it for this than in every
> fp op helper. What am I missing?
I'm not sure what you are suggesting.
If you are suggesting
void helper_reset_fpstatus(CPUPPCState *env)
{
- set_float_exception_flags(0, &env->fp_status);
+ assert(get_float_exception_flags(&env->fp_status) == 0);
}
then, sure, that works. But we also want to remove that call, so in order to
retain the check for debugging, we need to move the assert into the other
helpers.
r~
