xazax.hun added inline comments.

================
Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:49
+  // in tests.
+  std::set<const CFGBlock *> Preds;
   Preds.insert(Block.pred_begin(), Block.pred_end());
----------------
sgatev wrote:
> xazax.hun wrote:
> > xazax.hun wrote:
> > > Are we sure that the memory addresses of CFGBlocks are stable enough for 
> > > a deterministic order? Alternatively, we could use the block ids for the 
> > > ordering.
> > Also, could you describe where the flakiness is originated from? Naively, 
> > I'd expect that the order in which we process the predecessors should not 
> > change the results of the analysis.
> You're right, using block ids for ordering is better. I updated the code.
> 
> > Also, could you describe where the flakiness is originated from?
> 
> Say we have a block `B1` with predecessors `B2` and `B3`. Let the environment 
> of `B2` after evaluating all of its statements is `B2Env = { Expr1 -> Loc1 }` 
> and the environment of `B3` after evaluating all of its statement is `B3Env = 
> { Expr2 -> Loc2 }` where `ExprX -> LocX` refers to a particular mapping of 
> storage locations to expressions.
> 
> What we want for the input environment of `B1` is `{}` because `B2Env` and 
> `B3Env` do not contain common assignments of storage locations to 
> expressions. What we got before this patch is either `B2Env.join(B3Env) = { 
> Expr1 -> Loc1 }` or `B3Env.join(B2Env) = { Expr2 -> Loc2 }`.
> 
> Without deterministic ordering of predecessors the test that I'm introducing 
> in this patch is flaky.
> What we got before this patch is either B2Env.join(B3Env) = { Expr1 -> Loc1 } 
> or B3Env.join(B2Env) = { Expr2 -> Loc2 }.

I think I'm still missing something. With this patch, wouldn't both 
B2Env.join(B3Env) and B3Env.join(B2Env) produce the empty environment? If that 
is the case, do we still care about a deterministic order?





Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117754

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to