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
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to