@dexonsmith - any suggestions to move forward on this ?

@vsk / @dexonsmith - I've added some more info in latest comments. Let me know if I can provide more info / context on this to be able to reach a conclusion. Or if you think it is clear at this point that the hash-based approach is a no-go.

@dexonsmith I did a benchmark with the worst case example I can come up with - 1000 regular functions and 1000 blocks : https://paste.ofcode.org/CFU6b46nuAA6ymxUEpkzka
I didn't manage to measure any performance decrease (the performance decrease was within the noise of repeated runs) - so seems the hashing is insignificant compile-time-wise even in worst case scenario (where most of the code is in function blocks).
And here is the actual text that is being hashed - the textual representation of the AST, not the IR: https://paste.ofcode.org/bfMzhbvHz9HRT7mVMe48Mx

@vsk - about breaking existing workflows - I was referring only to if / when this gets shipped out as the default - all the names for the function blocks will change and this might cause issue with tooling that relies on symbol names being consistent across builds.

@vsk - sure will add tests when removing from RFC.
As for making it default - would rather have this under a flag as hashing the block contents does have some overhead and I imagine this feature wouldn't be beneficial in most scenarios. Also, unexpectedly (by default) changing the name of the function blocks might have a negative impact on some existing workflows.

I don't have commit access. steven_wu can you please commit this for me ?

Indeed it can, that is another fix. I was just doing the "obvious" fix since I assume there is a reason why the ::codegen(Module &TheModule) function exists.
Should we just remove the function altogether ? As-is it is not thread-safe = results in crashes.
Also, a clone of TMBuilder is not expensive considering the context.