steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land.
In D106823#3109469 <https://reviews.llvm.org/D106823#3109469>, @martong wrote: >> As of diff 5, line 1767 and all the code in the block at line 2184 are >> uncovered by the tests you provided. > > Thanks, I've added new tests that cover the re-assume logic (the block of > line 2184). > > However, I was unable to add a test that covers the case when the > simplification of a trivial symbol in the disequality info results an > infeasible state (line 1767). Here is how I tried: I had changed the line to > an assertion and then initiated the static analysis of the following > opensource projects: > memcached,tmux,curl,twin,redis,vim,openssl,sqlite,ffmpeg,postgresql,tinyxml2,libwebm,xerces,bitcoin,protobuf. > My idea had been, if the assertion would fired then I would use creduce to > the create the test case (btw, this is how I added the other infeasible state > test case). However, it did not fired. > > IMHO, having a defensive check at L1767 is correct b/c there is a slight > chance of reaching an infeasible state there. Although the chance is minimal, > I cannot prove that is 0. It's fine by me. Thanks for the investigation. In D106823#3111110 <https://reviews.llvm.org/D106823#3111110>, @martong wrote: > There are no runtime or peak memory usage growth with this patch (actually, > the runtime decreases with a few %). > I am attaching the measurements results of the latest Diff.F20089096: > stats.html <https://reviews.llvm.org/F20089096> Great! The tests refer to `reg_$0` by spelling the id number, which is unfortunate, but I suspect these tests will break for the slightest changes in the solver anyway so I'm not too bothered with this. Please wait a week for the rest of the members of the community to have a look before committing. @NoQ @Szelethus @ASDenysPetrov Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106823/new/ https://reviews.llvm.org/D106823 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
