BitFunnel / NativeJIT

A C++ expression -> x64 JIT
http://bitfunnel.org/
MIT License
1.14k stars 83 forks source link

JumpTable::PatchCallSites has dead code that seems intended to do something? #75

Open danluu opened 7 years ago

danluu commented 7 years ago
    void JumpTable::PatchCallSites()
    {
        for (size_t i=0; i < m_callSites.size(); ++i)
        {
            const CallSite& site = m_callSites[i];
            const uint8_t* labelAddress = AddressOfLabel(site.GetLabel());
            uint8_t* siteAddress = site.Site();
            ptrdiff_t delta = labelAddress - siteAddress - site.Size();

            // TODO: Evaluate whether special cases for size == 2 and size == 4 actually improve performance.
            size_t size = site.Size();
            if (size == 2)
            {
                LogThrowAssert(delta <= std::numeric_limits<int16_t>::max() &&
                               delta >= std::numeric_limits<int16_t>::min(),
                               "Overflow/underflow in cast to int16_t.");
                *(reinterpret_cast<int16_t*>(siteAddress)) = static_cast<int16_t>(delta);
                siteAddress += size;  // <----------- CODE DOES NOTHING
            }
            else if (size == 4)
            {
                LogThrowAssert(delta <= std::numeric_limits<int32_t>::max() &&
                               delta >= std::numeric_limits<int32_t>::min(),
                               "Overflow/underflow in cast to int32_t.");
                *(reinterpret_cast<int32_t*>(siteAddress)) = static_cast<int32_t>(delta);
                siteAddress += size;  // <----------- CODE DOES NOTHING
            }
            else
            {
                while (size > 0)
                {
                    *siteAddress++ = static_cast<uint8_t>(delta);
                    delta = delta >> 8;
                    size--;
                }
            }
        }
    }

siteAddress += size doesn't do anything. I haven't looked at this to figure out what the intent is. It's possible it would be fine to just delete these useless lines, but it's also possible that we have a bug here and something else was intended.