Re: [PATCH 04/15] arc: TCG and decoder glue code and helpers

[Shahab Vahedi <Shahab.Vahedi@synopsys.com>]

Hi Richard,

On 12/1/20 10:35 PM, Richard Henderson wrote:
> On 11/11/20 10:17 AM, cupertinomiranda@gmail.com wrote:
>> From: Cupertino Miranda <cmiranda@synopsys.com>
>> +void helper_enter(CPUARCState *env, uint32_t u6)
>> +{
>> +    /* nothing to do? then bye-bye! */
>> +    if (!u6) {
>> +        return;
>> +    }
>> +
>> +    uint8_t regs       = u6 & 0x0f; /* u[3:0] determines registers to save */
>> +    bool    save_fp    = u6 & 0x10; /* u[4] indicates if fp must be saved  */
>> +    bool    save_blink = u6 & 0x20; /* u[5] indicates saving of blink      */
>> +    uint8_t stack_size = 4 * (regs + save_fp + save_blink);
>> +
>> +    /* number of regs to be saved must be sane */
>> +    check_enter_leave_nr_regs(env, regs, GETPC());
> 
> Both of these checks could be translate time.
> 
>> +    /* this cannot be executed in a delay/execution slot */
>> +    check_delay_or_execution_slot(env, GETPC());
> 
> As could this.
> 
>> +    /* stack must be a multiple of 4 (32 bit aligned) */
>> +    check_addr_is_word_aligned(env, CPU_SP(env) - stack_size, GETPC());
>> +
>> +    uint32_t tmp_sp = CPU_SP(env);
>> +
>> +    if (save_fp) {
>> +        tmp_sp -= 4;
>> +        cpu_stl_data(env, tmp_sp, CPU_FP(env));
>> +    }
> 
> And what if these stores raise an exception?  I doubt you're going to get an
> exception at the correct pc.

I've added a few bad-weather test cases [1] and they work as expected. Indeed,
none of those tests trigger an exception during the "cpu_stl_data()". Could you
elaborate why you think the PC might be incorrect? Then I can add the corresponding
tests and fix the behavior.

[1]
https://github.com/foss-for-synopsys-dwc-arc-processors/qemu/blob/master/tests/tcg/arc/check_enter_leave.S#L227

> 5-10 tcg opcodes is the rule of thumb.  A conditional exception (requiring a
> branch) is a good reason to put the whole thing out of line.
> 
> In the case of enter or leave, this is one load/store plus one addition,
> followed by a branch.  All of which is encoded as fields in the instruction.
> Extremely simple.

You're suggesting that "enter/leave" should use TCG opcodes instead of helpers? If yes,
do you really think it is possible to implement each with ~10 opcodes?

-- 
Shahab