[PATCH] D93762: SCCP: Refactor SCCPSolver

[Sanne Wouda via Phabricator via llvm-commits]

sanwou01 added a comment.

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?



================
Comment at: include/llvm/Transforms/Scalar/SCCP.h:30
 #include "llvm/Transforms/Utils/PredicateInfo.h"
+#include "llvm/Transforms/Utils/SCCPSolver.h"
 
----------------
mivnay wrote:
> sanwou01 wrote:
> > Ideally, this include should not be needed.
> > 
> > Perhaps keep the definition of `struct AnalysisResultsForFn` in this file (as it is part of the `runIPSCCP` interface) and create a similar (but differently named) struct for the implementation detail of the `SCCPSolver`.
> `struct AnalysisResultsForFn` is part of the `SCCPSolver.AnalysisResults`. `runIPSCCP` just passes it on to the `SCCPSolver` via ` void SCCPSolver::addAnalysis(Function &F, AnalysisResultsForFn A)` interface. It would be compilation error to pass a different type. 
Ah fair enough; I still think we can break this dependency, but we can do that afterwards.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93762