[PATCH] D39982: [IRBuilder] Set the insert point and debug location together

Jonas Devlieghere via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 22 07:40:37 PDT 2018


JDevlieghere accepted this revision.
JDevlieghere added a comment.

In https://reviews.llvm.org/D39982#1010833, @vsk wrote:

> Ping, I'd love to find a path forward on this since it's a prerequisite for fixing bugs in LSR and SCEVExpander.
>  ...
>  Here are some ways I think we can move forward:
>
> - Rename the new APIs to 'setInsertPointAndLoc' and keep the old APIs in perpetuity.
> - Deprecate the old APIs for 1-2 releases and use the time to migrate in-tree users.


Assuming this is about the C++ APIs, I personally don't see any advantage of your first suggestion and I'd go with the current state of the diff. I also think it doesn't really matter all that much and certainly it's not worth blocking this review over. Either we fix all the in-tree uses by the next release in which case it doesn't matter, or we don't in which case nothing changes for downstream users. Sure, it might be confusing for the transition period, but that's true for both alternatives and at least it's better than what we currently have. Happy to hear other people's opinions if they disagree, but otherwise I'd love to see this getting checked in. As far as I'm concerned this LGTM.


https://reviews.llvm.org/D39982





More information about the llvm-commits mailing list