Re: [PATCH] warn when zero-extending a negation

[Luc Van Oostenryck <luc.vanoostenryck@gmail.com>]

On Wed, Dec 16, 2020 at 02:37:04PM -0800, Linus Torvalds wrote:
> On Wed, Dec 16, 2020 at 2:25 PM Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
> >
> > I suppose that it is fine for you that I your SoB instead of the
> > 'Originally-by' I used here?
> 
> Either works for me.
> 
> Some of the cases I saw (from my very quick look) were because of
> annoying zero extensions that should have been optimized away.

...
 
> Zero-extend, and then truncate:
> 
>   zext.32     %r2 <- (8) %arg1
>   shl.32      %r5 <- $1, %arg2
>   trunc.8     %r6 <- (32) %r5
> 
> then do the 'not' in 8 bits, because we did that part ok:
> 
>    not.8       %r7 <- %r6
> 
> and then the zero-extend and truncate thing again:
> 
>     zext.32     %r9 <- (8) %r7
>     and.32      %r10 <- %r2, %r9
>     trunc.8     %r11 <- (32) %r10
> 
> and then the return in 8 bits:
> 
>     ret.8       %r11
> 
> because sparse doesn't do the simplification to just do the shl and
> and in 8 bits (but sparse *does* do the simplification to do the 'not'
> in 8 bits).

But replacing a trunc + zext by the corresponding masking, very
little, if anything is done for such 'mixed-width' expressions.
So, I'm even a bit surprised by the not.8 but well ... 

I also confess, that coming from an ARM background, seeing a
not.8 or a shl.8 seems quite unnatural to me. I would tend to
force everything to at least the same width as 'int'.

> So the warning comes from the fact that we kept that zero extend
> around, even though it really wasn't relevant..
> 
> I don't know how many of the false positives were due to things like
> this, but at least a couple were.

Yes, most probably.
I suppose my (old old) series about bitfield simplification will
help a little bit here. I'll try to look at this during the holidays.

-- Luc