Hi Martin, Honza,
Thanks again for agreeing to mentor this. I've been looking into the
IPA-ICF comparison logic and wanted to share what I've found so far.
I went through sem_function::equals_wpa, func_checker and the variable
comparison code, and found roughly two dozen candidate categories:
properties that ICF either doesn't compare or only partially compares.
I've also been checking whether later passes actually consume those
properties, since that's where a mismatch can become a correctness
issue.
One case looks particularly concerning. It's related to PR123227, but
this time the attribute is on a call's function type:
typedef void fn (int *) __attribute__ ((nonnull));
int *volatile vp;
__attribute__ ((noipa)) void id (int *p) {}
void pa (int *p) { ((fn *) id) (p); id (p); id (p); }
void pb (int *p) { id (p); id (p); id (p); }
int use_b (int *q) { pb (q); return q == 0; }
int main (void) { return use_b (vp) != 1; }
At -O2 on trunk, this exits 1, while disabling IPA-ICF makes it exit 0.
The two functions are merged, and after inlining, VRP appears to use the
nonnull information from pa to eliminate the null check in use_b.
On GCC 13.3, the exact testcase exits 0, but a variant with use_b marked
noinline,noclone exits 1 with IPA-ICF and 0 without it.
The likely issue is in compare_gimple_call (ipa-icf-gimple.cc:729). It
uses comp_type_attributes, whose ordinary attribute comparison ignores
attributes that don't affect type identity, and nonnull is one of them.
I couldn't find an existing report for this call-fntype case.
I'm considering following the approach used in PR123227 and comparing
the call function-type attribute lists directly. I'm not sure whether we
should reject every attribute difference or only those that can affect
semantics or downstream optimization. Candidates include nonnull,
returns_nonnull, the allocation and alignment attributes, the internal
fn spec, and reproducible and unsequenced. I'd like to investigate this
rather than assume it's a complete list.
For the 350-hour project, I'm thinking of focusing on:
- Maintaining an inventory of these properties.
- Adding regression tests for the relevant cases.
- Fixing correctness issues identified by the audit.
- Investigating whether selected pre-ICF summaries, such as return
value ranges, need to be preserved.
- Testing multi-unit LTO cases, including differing options between
units.
- Measuring whether normalizing conservative cases is worthwhile
compared with simply declining the merge.
I'd appreciate your thoughts on a few design decisions:
1. Should we compare the complete call-fntype attribute lists, as in
PR123227, or use a narrower comparison?
2. How much merge-rate loss would be acceptable for a correctness fix?
For reference, the PR119006 fix declined 25 merges at -O2 and 29 at
-Os on a libstdc++ LTO build.
3. Would you prefer the inventory in source comments, documentation,
or the testsuite?
4. For properties that C tests cannot express, would GIMPLE FE tests
or a plugin be appropriate?
Thanks,
Rohith
