[FFmpeg-devel] [PATCH 1/5] lavu/common.h: Fix UB in av_clipl_int32_c()

[Tomas Härdin]

tor 2024-05-30 klockan 14:50 +0300 skrev Rémi Denis-Courmont:
> 
> 
> Le 30 mai 2024 12:40:20 GMT+03:00, "Tomas Härdin" <git at haerdin.se> a
> écrit :
> > tor 2024-05-30 klockan 09:41 +0300 skrev Rémi Denis-Courmont:
> > > Hi,
> > > 
> > > Le 30 mai 2024 01:13:14 GMT+03:00, "Tomas Härdin"
> > > <git at haerdin.se> a
> > > écrit :
> > > > The entire patchset passes FATE
> > > 
> > > Is the version in riscv/intmath.h safe? It looks to me that the
> > > GCC
> > > codegen for not only RV64 but also AArch{32,64} and x86-64 is
> > > better
> > > than this.
> > 
> > I haven't checked. It seems weird to me to have two different C
> > versions.
> 
> The common one ends up horrendously bad on RV, and presumably on MIPS
> and some other RISC ISA.
> 
> > We shouldn't rely on type punning.
> 
> Because?
> 
> We should depend on punning as long as it conforms to the standard.

My mistake, I forgot type punning is allowed in C. It's UB in C++

> > The standard compliant way
> > is to use memcpy()
> 
> That's way worse than union in terms of how proactively the compiler
> needs to optimise, and both approaches are as confirming.

A good compiler will do the same thing

Maybe I can get the riscv version covered by Eva as well. That's beyond
the scope of this patchset

/Tomas