On 03.06.2019, at 14:07, Andreas Rheinhardt <andreas.rheinha...@gmail.com> wrote:
> Reimar Döffinger: >> On 03.06.2019, at 00:37, Andreas Rheinhardt <andreas.rheinha...@gmail.com> >> wrote: >> >>> Up until now, a temporary variable was used and initialized every time a >>> value was read in CBS; if reading turned out to be successfull, this >>> value was overwritten (without having ever been looked at) with the >>> value read if reading was successfull; on failure the variable wasn't >>> touched either. Therefore these initializations can be and have been >>> removed. >>> >>> Signed-off-by: Andreas Rheinhardt <andreas.rheinha...@gmail.com> >>> --- >>> And? What did the ancient compilers say? >> >> Not sure how to read that, but some compilers probably produce "may be used >> uninitialized" warnings after this change. >> IMHO it would be better to need evidence of an advantage to remove variable >> initialization for non-trivial code, even if they are unnecessary. Your >> commit message doesn't seem to mention one at least. >> But I am happy to let some maintainer have the last word. > This comment refers to what Mark said in his review [1] of an earlier > version of this patchset. He explicitly mentioned that (if his memory > is right) an older compiler warned without the initializations. > And I actually thought that there is no need for a further reason to > remove unnecessary initializations despite the usual ones: Speed and > code size. E.g. the size of my cbs_mpeg2.o went down from 30421 B to > 29445 B after this patch; for cbs_h2645.o the numbers are 249605 B > and 242245 B. These macros are used a lot (mostly indirectly via other > macros); see the various cbs_*_syntax_template.c files for that. Well, your commit message did not mention that. I kind of assumed that given the compiler does not emit a "may be used uninitialized" warning it figured out that the initialization was unnecessary and would produce identical code. Maybe the compiler just is more conservative about that warning - or the code size increase might be a bug worth reporting. But I admit finding out which might be a bit too much work. Either way I would appreciate mentioning explicitly in the commit message code size and speed advantages if you have numbers already anyway. Thanks! Reimar _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
