[PATCH] D16795: WholeProgramDevirt: introduce.

Peter Collingbourne via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 9 13:38:49 PST 2016


pcc added inline comments.

================
Comment at: lib/Transforms/IPO/WholeProgramDevirt.cpp:603
@@ +602,3 @@
+  return true;
+}
+
----------------
joker.eph wrote:
>  I still regret that the design of the IR representation haven't been considered any further. In many other cases such a feature would be block on such consideration (Technical debt). As much as this is not widespread, it still involves changes in the frontend, so it is not completly isolated. It seems to me that CFI acts as a Trojan horse here. It is not clear to me if the actual "bitset" has any other justification for its existence that being the clever and efficient *runtime* system that CFI is using.  Now I can be totally wrong and it is possible that when CFI was integrated, the IR representation was carefully designed independently of the actual CFI runtime, I just don't have time to dig in the history.
I feel in general that it's best not to do too much design up-front in cases where it can be avoided, as that can lead to a sub-optimal design. In this case, we control the frontend, so it's certainly possible to reverse out of any bad decisions we make here. Instead, I feel it's best to revisit the design in response to feature requirements. For example, as I implement `llvm.bitset.check` and some of the ABI breaking features discussed on the proposal thread I expect I'll end up tweaking the design in response to them, and the ultimate design will be better for it.


http://reviews.llvm.org/D16795





More information about the llvm-commits mailing list