> > This seems good. Profile-arcs is rarely used by itself - most of time it > > is implied by -fprofile-generate and -ftest-coverage and since > > condition coverage is more associated to the second, I guess > > -fcondition-coverage is better name. > > > > Since -fcondition-coverage now affects gimplification (which makes me > > less worried about its ability to map things back to conditionals), it > > should be marked as incompatible with -fprofile-generate and > > -fprofile-use. It would only work with -fcondtion-coverage was used > > both with -fprofile-generate and -fprofile-use and I do not see much use > > of this. On the other hand I can imagine users wanting both coverage and > > -fprofile-generate run at once, so we should output sorry message that > > this is not implemented > > I don't quite understand this - gimplification is affected only in the sense > that slightly more information is recorded, why would it be incompatible > with -fprofile-generate?
You are right. Originally I tought you add extra temporary boolean variables (as described by the comment in condition coverage) but that happens only at tree-profile time. > > > > If condition statement is removed before profiling is done, you will end > > up with a stale entry in the hash table. > > We already keep EH regions and profile histogram in on-side hashtables, > > so I think you need to follow gimple_.*histograms calls in > > gimple-iterator.cc and be sure that the hashtable is updated. > > Also if function is inlined (which may be forced at -O0 using > > always_inline) the conditional annotations will be lost. > > > > I think you should simply make condition coverage to happen before > > pass_early_inline is run. But that may be done incrementally. > In the v8 revision I switched strategy to not using an auxiliary hash table, > but rather temporarily in the gimple.uid field until the basic block is > created and it can be stored there. What do you think about that approach? > > https://patchwork.sourceware.org/project/gcc/patch/20231212114125.1998866-...@lambda.is/ > > I assume the stale entries is why I got ICEs from what looked like double > frees in gc runs. I see, I missed version 8. Sorry for that. I am not sure about (ab)using gimple_uid - it looks like something that may lead to surprises later. We can check with Richi for an opinion. It seems to me that using startegy similar to histogram annotations may end up being more maintainable (since at least it is symetric to what we already have). I wonder if we want to have in longer term more general annotation infratstructure like we do for symbol table. I will check the version patch day after tomorrow, since now I am about to set off for new year celebration... Happy new year, Honza
