Hi Dave, Quick recap: this is the v2 series I sent on Sep 26 -- patches 0001-0004 and 0006. 0001-0003 are unchanged from what you approved. 0004 now has only the analyzer and xml.cc hunks; I dropped the gcov.cc and canvas.cc hunks. 0006 is the array2::set overload you suggested, so the std::move in canvas::paint now really moves. (Martin has already pushed the ipa-cp patch.)
Just a gentle ping on these -- let me know if anything needs changing, or if you're able to push them. Thanks, Linden On Sat, Sep 26, 2026 11:21 PM, best xmg <[email protected]> wrote: > Hi Dave, > > Thanks for the review. Attached are v2-0004 and v2-0006. > > v2-0004-gcc-analyzer-drop-std-move-calls-that-have-no-effect.patch.txt > now has only the analyzer and xml.cc hunks you approved. I dropped the > gcov.cc and canvas.cc hunks; canvas.cc keeps its std::move. > > You are right about the canvas.cc change. > v2-0006-text-art-add-array2-set-overload-taking-an-rvalue-el.patch.txt > is new, as you suggested: an array2::set overload taking element_t &&, > so that std::move in canvas::paint now really moves. > > The series bootstrapped (x86_64-pc-linux-gnu, --enable-checking=yes) > with no errors. > > Could you push patch 1-3 along with v2-0004 and v2-0006 for me? > I don't have commit access. > > If anything is not going well, please tell me; I would really appreciate > it. > > Thanks, > Linden > > On Fri, Sep 25, 2026 05:58 AM, David Malcolm <[email protected]> wrote: > >> On Thu, 2026-09-24 at 10:21 +1200, best xmg wrote: >> > Hi all, >> > >> > Reference and move semantics are a part of C++ that experienced >> > engineers >> > still get subtly wrong, so I audited the tree with clang-tidy's >> > performance-* checks, which trace whether a by-value parameter is >> > ever >> > mutated or a std::move target is actually reachable -- the kind of >> > exhaustive per-parameter check regular compiler warnings and human >> > review >> > aren't built to do. Only patch 1 has a measured performance number; >> > patches 2-5 are correctness/consistency fixes in the same spirit with >> > no >> > individual perf claim. No functional change is intended anywhere in >> > the >> > series. >> > >> > Patch 1 removes a needless deep-copy of program_state (which owns a >> > whole >> > region_model) when -fanalyzer creates an exploded_node, by giving >> > program_state a move-assignment operator and threading the move >> > through >> > point_and_state and exploded_node. >> > >> > Testing: >> > - x86_64-pc-linux-gnu, --enable-languages=c,c++,lto, --disable- >> > bootstrap. >> > - check-gcc analyzer.exp/tree-ssa.exp/ipa.exp on current trunk >> > (492fbdbf99d): 16820+10537 pass. Same 2 pre-existing failures as >> > unpatched trunk, unrelated to this series: >> > * omp-parallel-for-1.c: -Wanalyzer-symbol-too-complex depth boundary. >> > * ssa-sink-18.c: known r17-3756 regression (unrelated ivopts change). >> > - Timing (patch 1 only): 8 interleaved `cc1 -fanalyzer` runs on >> > libiberty/cp-demangle.c, binary swapped each run: 63.98s -> 60.10s >> > median (6.06% faster), no overlap across all 8 pairs. >> >> Thanks, nice to see a measurable performance win in the analyzer for >> this. >> >> Did you try a bootstrap of GCC with the patches? >> >> Taking the patches one by one: >> >> "[PATCH 1/5] analyzer: avoid deep-copying program_state when creating >> an exploded_node": looks good to me. >> >> "PATCH 2/5] analyzer, diagnostics: add missing std::move for by-value >> sinks": looks good to me. >> >> "[PATCH 3/5] diagnostics: take HTML tag names as const char * in >> source-printing": looks good to me >> >> "[PATCH 4/5] gcc, analyzer: drop std::move calls that have no effect": >> * the analyzer/*.cc changes look good to me >> * I can't speak to the gcov.cc change >> * gcc/text-art/canvas.cc: looks like it would be better to implement an >> overload of text_art::array2::set that takes an element_t &&, and keep >> the std::move here, since copying styled_unichar instances is non- >> trivial >> * the xml.cc change looks good to me >> >> I can't speak to patch 5. >> >> Thanks again >> Dave >> >> >> >> >> >> >> > >> > Full test logs and the benchmark script are available on request. >> > >> > I don't have push access; posting for review. Happy to split patch 1 >> > out on its own if that's preferred, or to fold 2-5 together -- >> > they're >> > independent of each other and of patch 1. >> > >> > The five patches are attached as plain-text files. >> > >> > linden (5): >> > analyzer: avoid deep-copying program_state when creating an >> > exploded_node >> > analyzer, diagnostics: add missing std::move for by-value sinks >> > diagnostics: take HTML tag names as const char * in source-printing >> > gcc, analyzer: drop std::move calls that have no effect >> > range-op, tree-ssa-ccp, fold-const, ipa-cp: bind wide_int / >> > widest_int by >> > reference >> > >> > gcc/analyzer/access-diagram.cc | 2 +- >> > gcc/analyzer/engine.cc | 17 ++++++++++++----- >> > gcc/analyzer/exploded-graph.h | 16 ++++++++++++++++ >> > gcc/analyzer/infinite-loop.cc | 2 +- >> > gcc/analyzer/program-state.cc | 27 +++++++++++++++++++++++++++ >> > gcc/analyzer/program-state.h | 1 + >> > gcc/diagnostics/html-sink.cc | 4 ++-- >> > gcc/diagnostics/source-printing.cc | 27 ++++++++++++--------------- >> > gcc/fold-const.cc | 9 +++++---- >> > gcc/fold-const.h | 8 ++++---- >> > gcc/gcov.cc | 2 +- >> > gcc/ipa-cp.cc | 9 ++++++--- >> > gcc/ipa-cp.h | 6 +++--- >> > gcc/range-op.cc | 10 +++++----- >> > gcc/text-art/canvas.cc | 2 +- >> > gcc/tree-ssa-ccp.cc | 5 +++-- >> > gcc/xml.cc | 2 +- >> > 17 files changed, 101 insertions(+), 48 deletions(-) >> > >> > Regards, >> > Linden >> >>
