[lld] r233088 - [Mips] Suppress "right shift by too large amount" warning

[David Blaikie]

On Wed, Mar 25, 2015 at 10:38 AM, Simon Atanasyan <simon at atanasyan.com>
wrote:

> On Wed, Mar 25, 2015 at 8:30 PM, David Blaikie <dblaikie at gmail.com> wrote:
> >
> > On Wed, Mar 25, 2015 at 6:20 AM, Simon Atanasyan <simon at atanasyan.com>
> > wrote:
> >>
> >> I hope the problem has been fixed by r233186. Unfortunately using
> >> getRInfo does not solve the problem because in 32-bit case we extract
> >> too much data and touch symbol index part of r_info field.
> >>
> >> So I just factor out the code that gets the "tag" into the separate
> >> function. That allows to keep all bit manipulation code into the
> >> single place and removes the warning.
> >
> >
> > This (to me - though lld isn't my project, etc) just seems to complicate
> the
> > code further - what's the point of &ing the 0xFFFFFF00 mask if you're
> just
> > shifting down 8 anyway? This hides the issue from the compiler because
> the
> > bitwise & does integer promotion (so the char is promoted to int, then
> > and-ed, then shifted, so the shift is over an int instead of a char) but
> > produces exactly the same result & now makes the optimizer (& reader) do
> a
> > bit more work?
>
> The general goal is to extract high 24 bits of r_info field. I think
> using the 0xffffff00 mask is a good method to state this goal.
> Shifting right by 8 bits is just an attempt to simplify the following
> usage of the _tag field.
>

Sure - but the usual code one would expect is to keep it shifted down and
to populate it by shifting down. The extra masking seems
surprising/inexplicable, since it doesn't do anything (removing it & the
code has exactly the same result) - it'd make a reader do a double take
wondering whether this code is somehow meaningful/significant/observable
when it isn't.


> By the way, what do you suggest to silence the VC++ warning?
>

Disabling the warning, this is a false positive because some instantiations
of the template produce non-zero results out of this shift and the fact
that some other instantiations always produce 0 is fine. Same goes for
template-conditionally dead code, etc.

- David


>
> --
> Simon Atanasyan
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150325/9c834769/attachment.html>