Refactor iterative analysis code to common method #3773
Conversation
|
Thank you for your contribution, this PR is tracked internally by GR-33705 |
|
|
||
| if (stopIterationCriteria.getAsBoolean()) { | ||
| if (numTypes != universe.getTypes().size() || numMethods != universe.getMethods().size() || numFields != universe.getFields().size()) { | ||
| reportError.accept( |
There was a problem hiding this comment.
I think this error reporting code should remain in NativeImageGenerator. That avoids passing in a reportError handler. And duringAnalyssiAction and stopIterationCriteria can be combined to a single Predicate (i.e., boolean function). That way only one lambda needs to be passed into this method, making it maintainable.
There was a problem hiding this comment.
Fixed as suggested.
ziyilin
force-pushed
the
iterativelyAnalysis
branch
2 times, most recently
from
3add7df to
864222d
Compare
| int numMethods = aUniverse.getMethods().size(); | ||
| int numFields = aUniverse.getFields().size(); | ||
|
|
||
| String msg = ((NativeImagePointsToAnalysis) bb).iterativelyAnalyze(debug, |
There was a problem hiding this comment.
I would call the method runAnalysis and add it to the BigBang interface. This would avoid the cast here and allow instantiation of other analysis types.
There was a problem hiding this comment.
Fixed as suggested.
| } | ||
| return !config.getAndResetRequireAnalysisIteration(); | ||
| }); | ||
| if (msg.length() > 0) { |
There was a problem hiding this comment.
I don't like this approach of returning a String on error. Instead you could just throw an com.oracle.graal.pointsto.util.AnalysisError from PoinstToAnalysis.runAnalysis() then catch it here and wrap it in a user error, something like: throw UserError.abort(original, "Analysis step failed. Reason: %s.", original.getMessage());
There was a problem hiding this comment.
fixed as suggested
| @@ -779,6 +744,8 @@ private boolean runPointsToAnalysis(String imageName, OptionValues options, Debu | |||
| return false; | |||
| } | |||
|
|
|||
|
|
|||
|
|
|||
There was a problem hiding this comment.
These extra lines should be removed.
ziyilin
force-pushed
the
iterativelyAnalysis
branch
2 times, most recently
from
a8ad963 to
5d03edc
Compare
Extract the iteratively analyzing code from NativeImageGenerator to PointsToAnalysis so it can be used by both native image generation and standalone pointsto analysis.
ziyilin
force-pushed
the
iterativelyAnalysis
branch
from
5d03edc to
01f82d3
Compare
Extract the iteratively analyzing code from
NativeImageGeneratortoPointsToAnalysisso it can be used by both native image generation and standalone pointsto analysis.This PR is related with Feature request: #3418