Skip to content

COMP: Use pzero() init for Packet locals in SelfadjointMatrixVector loop#6173

Merged
hjmjohnson merged 1 commit intoInsightSoftwareConsortium:mainfrom
hjmjohnson:eigen3-init-loadu-locals
May 1, 2026
Merged

COMP: Use pzero() init for Packet locals in SelfadjointMatrixVector loop#6173
hjmjohnson merged 1 commit intoInsightSoftwareConsortium:mainfrom
hjmjohnson:eigen3-init-loadu-locals

Conversation

@hjmjohnson
Copy link
Copy Markdown
Member

Follow-up to #6166 covering the additional GCC -O3 IPA-CP false-positive -Wmaybe-uninitialized warnings exposed when #6164 lands its ubuntu-24.04 + GCC 13/14 + C++20 toolchain. Mirrors #6166's pzero(Packet{}) pattern at the inner-loop Packet load sites.

Symptom on PR #6164

ITK.Linux.Python build at SHA bf6e691b8 (CDash buildid 1124-2153): 0 compile errors, 0 test failures, 4 warnings — all 4 in Modules/ThirdParty/Eigen3/src/itkeigen/Eigen/src/Core/products/SelfadjointMatrixVector.h:231 of the form

SelfadjointMatrixVector.h:231: warning: 'result_90' may be used uninitialized [-Wmaybe-uninitialized]
SelfadjointMatrixVector.h:231: warning: '_28' may be used uninitialized [-Wmaybe-uninitialized]

with GCC's note by argument 3 of type 'const float *' to 'run.constprop' declared here. report_build_diagnostics.py treats warnings as fatal → exit 255.

The Continuous build of main at the same toolchain-pre-#6164 baseline (Linux-Build14631-main-Python) shows warn=0. The toolchain bump in #6164 (ubuntu-22.04ubuntu-24.04, C++17 → C++20) is what exposes the additional IPA-CP cloning paths.

Why this works

pzero(Packet{}) is visible to GCC's IPA-CP as a definite zero starting state for the Packet SSA value. A bare Packet Bi = ploadu<Packet>(rhsIt); is opaque — GCC's IPA-CP-cloned run.constprop.isra can trace the SSA value back through the cloned signature's argument 3 (the rhs pointer in the clone after constant args are stripped) and decide along some specialized path the read is from uninitialized memory. The added Packet X = pzero(Packet{}); X = pload...; pattern gives IPA-CP a known starting state for A0i, A1i, Bi, and Xi — same trick #6166 used for the ptmp2/ptmp3 accumulators.

How to verify

This file's warning surface is only triggered on the toolchain combination embedded in #6164 (ubuntu-24.04 + C++20). Local verification on macOS does not reproduce. CI verification:

  1. Merge this PR.
  2. Rebase COMP: Minimize redundant Azure CI matrix (issue #6163) #6164 on top of new main.
  3. Confirm ITK.Linux.Python lands clean on rebased COMP: Minimize redundant Azure CI matrix (issue #6163) #6164.

Follow-up to InsightSoftwareConsortium#6166. On ubuntu-24.04 + GCC 13/14 + C++20 (the toolchain
that InsightSoftwareConsortium#6164 introduces), GCC -O3 IPA-CP emits additional false-positive
-Wmaybe-uninitialized at the outer selfadjoint_product_impl::run()
call site, attributing the read to "argument 3 of type const float *"
of the cloned selfadjoint_matrix_vector_product::run.constprop.isra.

Add pzero(Packet{}) declare-then-assign init for the four Packet
locals (A0i, A1i, Bi, Xi) loaded from the lhs/rhs/res pointers inside
the inner loop. Mirrors the InsightSoftwareConsortium#6166 ptmp2/ptmp3 pattern: pzero() is
visible to GCC's IPA-CP as a definite zero starting state, while a
bare declaration plus subsequent ploadu/pload is opaque and lets the
optimizer trace an uninit path through cloned variants.
@github-actions github-actions Bot added type:Compiler Compiler support or related warnings area:ThirdParty Issues affecting the ThirdParty module labels Apr 30, 2026
@hjmjohnson hjmjohnson marked this pull request as ready for review April 30, 2026 02:13
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 30, 2026

Greptile Summary

This minimal COMP: patch adds pzero(Packet{}) initialization before each ploadu/pload call for the four inner-loop Packet locals (A0i, A1i, Bi, Xi) in the selfadjointMatrixVectorProduct kernel, mirroring the same technique already applied to ptmp2/ptmp3 in PR #6166. The goal is to suppress four GCC 13/14 false-positive -Wmaybe-uninitialized warnings exposed by IPA-CP function cloning under -O3 + C++20, which bypasses the existing file-wide #pragma GCC diagnostic ignored guard.

Confidence Score: 5/5

Safe to merge — semantically no-op change that gives GCC's IPA-CP analysis a definite zero starting state, preventing false-positive warnings without altering runtime behavior.

Single-file change with 8 added lines; all four pzero-initialized values are immediately overwritten so there is no semantic difference. The pattern is identical to the already-merged #6166 fix for ptmp2/ptmp3 in the same loop, and the file-wide diagnostic pragma remains intact as a secondary guard. No logic, interface, or data changes.

No files require special attention.

Important Files Changed

Filename Overview
Modules/ThirdParty/Eigen3/src/itkeigen/Eigen/src/Core/products/SelfadjointMatrixVector.h Adds pzero(Packet{}) before four ploadu/pload calls in the vectorized inner loop to suppress IPA-CP false-positive -Wmaybe-uninitialized warnings with GCC 13/14 + C++20; semantically neutral since each zero-initialized value is immediately overwritten.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["GCC 13/14 + -O3 + C++20\nIPA-CP cloning of run()"] --> B["Creates run.constprop.isra\nspecialized clone"]
    B --> C{"SSA analysis of\nPacket locals"}
    C -->|"Before fix: opaque load\nPacket A0i = ploadu(a0It)"| D["IPA-CP cannot prove\nSSA value initialized\nalong cloned path"]
    D --> E["-Wmaybe-uninitialized\nwarning at line 231\nbypasses #pragma guard"]
    C -->|"After fix:\nPacket A0i = pzero(Packet{})\nA0i = ploadu(a0It)"| F["IPA-CP sees definite\nzero starting state\nfor SSA value"]
    F --> G["No false-positive warning\ngenerated in clone"]
    E --> H["report_build_diagnostics.py\ntreats warning as fatal → exit 255"]
    G --> I["Clean CI build ✓"]
Loading

Reviews (2): Last reviewed commit: "COMP: Use pzero() init for Packet locals..." | Re-trigger Greptile

Copy link
Copy Markdown
Member

@dzenanz dzenanz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good on a glance.

@hjmjohnson hjmjohnson marked this pull request as draft April 30, 2026 15:55
@hjmjohnson
Copy link
Copy Markdown
Member Author

@dzenanz Please delay merging this for the moment. This whack-a-mole problem, being solved by backporting eigen 5.0.1 fixes to the ITK-vendored 3.4.90 codebase, is creeping from "small acceptable bandage" to "large maintenance nightmare surgery".

I'm looking into updating the internal vendored eigen to 5.0.1 (with USE_SYSTEM_EIGEN back support to eigen 3.3.0 or 3.4.0) as an alternative.

@hjmjohnson hjmjohnson marked this pull request as ready for review May 1, 2026 17:01
@hjmjohnson hjmjohnson merged commit 1f7150e into InsightSoftwareConsortium:main May 1, 2026
12 of 17 checks passed
@hjmjohnson hjmjohnson added this to the ITK 6.0 Release Candidate 1 milestone May 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:ThirdParty Issues affecting the ThirdParty module type:Compiler Compiler support or related warnings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants