salman-javed-nz added a comment. Thank you very much to both of you for having a look at the patch. Yes, I agree it is a large patch, and I could have done a better job splitting it up into more manageable chunks.
One reason this change is so big is because I set myself an ambitious target for the performance gains I wanted to achieve in this patch. After spending more time with the current implementation in `master`/`main`, I began to realize how expensive it really is. Others have too, as demonstrated by the fact that multiple commits have been made to disable the functionality in downstream projects. So wherever I saw a more efficient way of doing things, I didn't want to rule out making a change, even if it means parts of the code had to be refactored or rewritten. There is a balance I have to strike between achieving my primary objective and minimizing the impact to the code base, and I admit I haven't got it right so far. We can discuss where exactly the balance should be, but let me tidy up the more egregious instances of unnecessary code first, like the public accessors that don't hold their weight that you mentioned. To make your lives easier, I will address your requested changes as separate updates to this patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116085/new/ https://reviews.llvm.org/D116085 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
