JitCache cleanup

[magumagu]

vector::insert() is way too slow in debug builds to be usable. Just write out the bit manipulation using raw pointers, since that's apparently the only way to get it right.

Get rid of ClearSafe() because the existing Clear() works just fine for the same purpose.

[delroth]

-1 from me. I care less about debug builds performance than code readability.

It sounds like we'll have to end up rewriting an optimized bitfield class. Thanks MSVC for sucking so much once again.

[lioncash]

I pretty much fall in-line with what delroth stated. Code readability is much more important (in my opinion, of course)

[magumagu]

Okay; updated version with the bit-manipulation extracted into a class.

[magumagu]

I think I've addressed all the review comments.

And yes, it's 2MB, but this needs to be fast; if we do anything complicated, it defeats the purpose of the optimization. We could probably reduce the granularity a bit: I mean, nothing would notice the difference if InvalidateICache invalidated, for example, 256 bytes rather than 32 bytes.

On a side note, we actually shouldn't be masking off the top three bits, because JIT generated code depends on the full value of the program counter... but fixing that isn't a high priority.

[magumagu]

Okay, this time I've really addressed all review comments.

[lioncash]

Looks good to me.

[delroth]

LGTM, please squash the commits that should be squashed together.

[shuffle2]

What is some example of this bad codegen from std::bitset?

[magumagu]

https://gist.github.com/kayru/753db9607d680e26194b

[shuffle2]

Eh, so the bitset issue is only in debug as well?
http://msdn.microsoft.com/en-us/library/aa985982.aspx
http://msdn.microsoft.com/en-us/library/aa985965.aspx
You could disable it only in this scope if you wanted.

[galop1n]

The visual implementation of the bit reference use internally the test and set methods that throw on invalid bounds. But the standard do not specify that the operator[] that returns that reference had to throw, that's a Microsoft bug. Technically not a bug, as operator[] on an invalid bounds is undefined behavior and throw is an valid outcome. But it is not coherent with operator[] of std::array and std::vector

To shuffle : sadly, that's not the range checked iterator :(

[shuffle2]

I see. The overhead that remains in release builds is because of callers to std::bitset::_Xran() in msvc's implementation. This is because bool operator[](size_t _Pos) const internally calls test, which can throw. reference operator[](size_t _Pos) doesn't do that. However, there are a few other paths which may end up at _Xran() as well :(

[Sonicadvance1]

This LGTM, is it ready for merging?
@delroth @shuffle2

[shuffle2]

It's fine how it is, although I'm slightly curious how codegen from supporting 64bit elements changes things (templatize underlying type by platform?).