> On Apr 8, 2025, at 21:13, Siddhesh Poyarekar <siddh...@gotplt.org> wrote: > > On 2025-04-08 15:22, Qing Zhao wrote: >> Changing a pointer reference to a call to .ACCESS_WITH_SIZE will impact the >> compiler optimization in two aspects: >> 1. The new call site might become a barrier that prevents code movement >> around it. > > Yeah, it's not a real problem IMO; it should only prevent code movement that > references the struct and the size expression, which is what we want anyway. > >> 2. Increased insn counts might impact the compiler heuristics that depends >> on code size. > <snip> >> For 2, When generating .ACCESS_WITH_SIZE to FAM references currently, one >> reference REF will become: >> (*.ACCESS_WITH_SIZE (REF, COUNTED_BY_REF, 1, (TYPE_OF_SIZE)0, -1, >> (TYPE_OF_ARRAY *)0)) >> It increase the # of IRs, but not that much. >> However, when generating .ACCESS_WITH_SIZE to a pointer REF to a structure >> with FAM, one reference will become: >> If (!type_unsigned (typeof (ref->c)) >> ref->c = (ref->c < 0) ? 0 : ref->c; >> sz_exp = MAX (sizeof (struct S), offset (struct S, a) + ref->c * sizeof >> (char)); >> ref = .ACCESS_WITH_SIZE (&ref, &sz_exp, 0, …); >> It’s obviously that the # if IRs will increase quite a lot for one pointer >> REF. So, I am a little concern on this impact. > > Isn't the only *real* addition the size expression? The .ACCESS_WITH_SIZE > should be zero cost in this context since it shouldn't add any actual code. After expand phase, .ACCESS_WITH_SIZE will be replaced by its first argument. And all the size expression before the call to .ACCESS_WITH_SIZE might be dead code eliminated later in the RTL phases.
So, I don’t think there will be run-time overhead concern. However, the IR size increase in the TREE optimization phase might impact the Optimization heuristics that base on IR size, for example, inlining decision, etc. That’s the major concern for me. > > A good measure of this could be to get a sense of additional coverage one > could get for __bdos with this change vs without it for, e.g. the kernel. > You could maybe use fortify-metrics[1] to find that. Why there will be additional coverage with this change? (This change means to replace pointer reference with structure type with FAM by a call to .ACCESS_WITH_SIZE in C FE) Based on my current patch (expand the counted_by refs to size expressions by demand for __bdos), All the __bdos should be covered, right? thanks. Qing > > Thanks, > Sid > > [1] https://github.com/siddhesh/fortify-metrics
