[PATCH] D93762: SCCP: Refactor SCCPSolver

[Vinay Madhusudan via Phabricator via llvm-commits]

mivnay added a comment.

In D93762#2557117 <https://reviews.llvm.org/D93762#2557117>, @fhahn wrote:

> In D93762#2549267 <https://reviews.llvm.org/D93762#2549267>, @sanwou01 wrote:
>
>> This looks good to me. @fhahn are you happy with the suggested approach to tighten up the SCCPSolver class in subsequent patches, when function inlining starts using it?
>
> IMO we should nail down the interface first,  rather than starting with exposing/moving everything, to avoid a lot of unnecessary churn.



> I think it makes sense to expose this more widely in general. But could you share the places where you intend to make use of it?

@fhahn `SCCPSolver` is hidden inside the `SCCP.cpp` file. I could just call the interface functions provided in the `SCCP.h` files and use it in my pass. But, it will be super in-efficient as I will be building the Solver repeatedly with almost same set of already computed lattice values. Wouldn't it be better to expose the utility to other users as you also agreed before?

> rather than starting with exposing/moving everything, to avoid a lot of unnecessary churn.

`SCCPSolver` is a single class. Do you want me to move some of the functions to private / protected mode? If so, isn't it better to do that in a different review where changes are easier to track?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93762/new/

https://reviews.llvm.org/D93762