steakhal added a comment.
I see. Simplification is always good. Let's measure and compare the runtime
characteristics before moving forward.
================
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
----------------
Emphasize what you mean by //relaxation//. You meant probably something like
//not replacing the complex symbol, just adding the simplified version to the
class//.
================
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
----------------
nor memory consumption
================
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);
----------------
Could you please elaborate on this in the review? I don't get the reasoning. I
might miss something.
================
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: {
----------------
Please try to omit unnecessary changes.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D114619/new/
https://reviews.llvm.org/D114619
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
