martong added inline comments.
================
Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:2226
+ //
+ // Empirical measurements show that if we relax assumption G then the
+ // runtime does not grow noticeably. This is most probably because the
----------------
steakhal wrote:
> Emphasize what you mean by //relaxation//. You meant probably something like
> //not replacing the complex symbol, just adding the simplified version to the
> class//.
ok
================
Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:2227
+ // Empirical measurements show that if we relax assumption G then the
+ // runtime does not grow noticeably. This is most probably because the
+ // cost of removing the simplified member is much higher than the cost of
----------------
steakhal wrote:
> nor memory consumption
ok
================
Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:2228-2229
+ // runtime does not grow noticeably. This is most probably because the
+ // cost of removing the simplified member is much higher than the cost of
+ // simplifying the symbol.
State = reAssume(State, ClassConstraint, SimplifiedMemberVal);
----------------
steakhal wrote:
> Could you please elaborate on this in the review? I don't get the reasoning.
> I might miss something.
It's not a reasoning, just a guessing. Another reason could be that simply the
time we spend here is negligible compared to the time we spend e.g. in
removeDeadBindings. (Actually, removeDeadBindings is very hot and takes
roughly 15-20 % of the runtime.) Perhaps I could do a measurement with a
sampling profiler to be able to provide a precise explanation to this. But I
think the point is that the runtime/memory consumption do not grow and the
concrete reason is secondary.
However, you are right, I should not get into guessing here, I'll just remove
this sentence.
================
Comment at: clang/test/Analysis/symbol-simplification-disequality-info.cpp:15-26
+ // CHECK: "disequality_info": [
+ // CHECK-NEXT: {
+ // CHECK-NEXT: "class": [ "((reg_$0<int a>) + (reg_$1<int b>)) +
(reg_$2<int c>)" ],
+ // CHECK-NEXT: "disequal_to": [
+ // CHECK-NEXT: [ "reg_$3<int d>" ]]
+ // CHECK-NEXT: },
+ // CHECK-NEXT: {
----------------
steakhal wrote:
> Please try to omit unnecessary changes.
I've made this change intentionally, so all hunks for each simplification steps
are indented similarly. E.g., after the `CHECK-NEXT:` string comes 3 space
characters until we the `{` character, see L16 and L34 and L51. Actually, I've
made an indentation error in a previous patch, which I though I can correct now.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D114619/new/
https://reviews.llvm.org/D114619
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
