MIPT-ILab / mipt-mips

Cycle-accurate pre-silicon simulator of RISC-V and MIPS CPUs
http://mipt-ilab.github.io/mipt-mips/
MIT License
341 stars 139 forks source link

using noexcept in potentially throwing functions #1459

Closed graudtv closed 3 years ago

graudtv commented 3 years ago

DataBypass class has a lot of methods, marked as noexcept, like this: https://github.com/MIPT-ILab/mipt-mips/blob/63eb9937e2b4a25ca0deceb0dfe2897b26a45e3a/simulator/modules/decode/bypass/data_bypass.h#L30-L34 Although, there is absolutely no guarantee, that they won't throw, because invocations like instr.get_src() are not noexcept. Why are we using it? It makes code non reusable and not exception-safe

pavelkryukov commented 3 years ago

there is absolutely no guarantee, that they won't throw

Could you please prove your statement? Up to my knowledge, correct input is guaranteed by our unit tests suite.

Why are we using it?

This method is a GPROF bottleneck, so we remove exception handling code to optimize more aggressively.

graudtv commented 3 years ago

This will fail to compile:

        auto is_in_RF( const Instr& instr, size_t src_index) const noexcept
        {
            static_assert(noexcept(instr.get_src( src_index)), "instr.get_src( src_index) may throw");
            const auto reg_num = instr.get_src( src_index);
            return get_entry( reg_num).current_stage.is_in_RF();
        }

And this too

    void RegisterStage::inc() noexcept
    {
        static_assert(noexcept(value + 1_lt), "Latency::operator +() is not noexcept too");
        value = value + 1_lt;
    }

So we are relying on some implicit thoughts about operations behaviour

pavelkryukov commented 3 years ago

I don't get it. Why should we add these static assertions?

We are relying on some implicit thoughts about operations behaviour

These thoughts are explicit, since they are defined in unit tests. Please correct me if I'm wrong.

I suspect you are trying to sell me "defensive programming" paradigm, where each function or procedure assumes the inputs may be incorrect. MIPT-V does not use it. Instead, we prove there are no chances to generate these incorrect inputs.

graudtv commented 3 years ago

Ok, I understood, thanks for answer