https://github.com/martinboehme closed
https://github.com/llvm/llvm-project/pull/72850
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
martinboehme wrote:
Abandoning in favor of #72985.
https://github.com/llvm/llvm-project/pull/72850
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
martinboehme wrote:
> +1 for the simpler approach. I have a high-level question about the stability
> of the locations, its effect on convergence.
Answered in the inline comment.
> But I am happy with the `ExprToVal` part of the simple patch.
SG! Then let's continue the discussion on #72985?
https://github.com/martinboehme edited
https://github.com/llvm/llvm-project/pull/72850
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -311,7 +318,10 @@ computeBlockInputState(const CFGBlock ,
AnalysisContext ) {
}
}
- JoinedStateBuilder Builder(AC);
+ // When performing the join, only retain state for those expressions that are
+ // consumed by this block. This avoids performing joins and
Xazax-hun wrote:
+1 for the simpler approach. I think I have a high-level question about the
stability of the locations, its effect on convergence. But I am happy with the
`ExprToVal` part of the simple patch.
https://github.com/llvm/llvm-project/pull/72850
https://github.com/Xazax-hun edited
https://github.com/llvm/llvm-project/pull/72850
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -311,7 +318,10 @@ computeBlockInputState(const CFGBlock ,
AnalysisContext ) {
}
}
- JoinedStateBuilder Builder(AC);
+ // When performing the join, only retain state for those expressions that are
+ // consumed by this block. This avoids performing joins and
martinboehme wrote:
After some internal discussion with @ymand, I think I'd like to retract this PR
in favor of https://github.com/llvm/llvm-project/pull/72985.
I will still keep this PR open for the time being however, until we've resolved
the discussion.
martinboehme wrote:
Update: Here's a draft implementation of the "simpler, bolder approach"
referenced above:
https://github.com/llvm/llvm-project/pull/72985
The code is a _lot_ simpler -- it's mainly deleting code -- and I think I'm
convinced at this point that this is the preferable thing
https://github.com/martinboehme edited
https://github.com/llvm/llvm-project/pull/72850
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/martinboehme edited
https://github.com/llvm/llvm-project/pull/72850
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -311,7 +318,10 @@ computeBlockInputState(const CFGBlock ,
AnalysisContext ) {
}
}
- JoinedStateBuilder Builder(AC);
+ // When performing the join, only retain state for those expressions that are
+ // consumed by this block. This avoids performing joins and
martinboehme wrote:
I'm intentionally not responding to comments in the code for now -- wanted to
wrap up this high-level discussion first.
> Overall seemed good (mostly just piping), but I think we need more
> explanation (on the review thread and somewhere appropriate in the code) of
>
@@ -52,23 +52,39 @@ class ControlFlowContext {
return StmtToBlock;
}
+ /// Returns the expressions consumed by `Block`. These are all children of
+ /// statements in `Block` as well as children of a possible terminator.
+ const llvm::DenseSet *
+
@@ -311,7 +318,10 @@ computeBlockInputState(const CFGBlock ,
AnalysisContext ) {
}
}
- JoinedStateBuilder Builder(AC);
+ // When performing the join, only retain state for those expressions that are
+ // consumed by this block. This avoids performing joins and
@@ -24,25 +24,35 @@
namespace clang {
namespace dataflow {
-/// Returns a map from statements to basic blocks that contain them.
-static llvm::DenseMap
-buildStmtToBasicBlockMap(const CFG ) {
- llvm::DenseMap StmtToBlock;
+/// Builds maps:
+/// - From statements to basic
@@ -256,8 +259,12 @@ class JoinedStateBuilder {
// to enable building analyses like computation of dominators that
// initialize the state of each basic block differently.
return {AC.Analysis.typeErasedInitialElement(), AC.InitEnv.fork()};
+
if
https://github.com/ymand requested changes to this pull request.
Overall seemed good (mostly just piping), but I think we need more explanation
(on the review thread and somewhere appropriate in the code) of what exactly
determines whether an expression is "needed".
I was wondering, when
https://github.com/ymand edited https://github.com/llvm/llvm-project/pull/72850
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
llvmbot wrote:
@llvm/pr-subscribers-clang-analysis
@llvm/pr-subscribers-clang
Author: None (martinboehme)
Changes
This has two major benefits:
* We avoid performing joins on boolean expressions and hence extending the flow
condition in cases where this is not needed. Simpler flow
https://github.com/martinboehme created
https://github.com/llvm/llvm-project/pull/72850
This has two major benefits:
* We avoid performing joins on boolean expressions and hence extending the flow
condition in cases where this is not needed. Simpler flow conditions should
reduce the
22 matches
Mail list logo
