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

[David Blaikie]

On Tue, Mar 24, 2015 at 9:22 AM, Aaron Ballman <aaron at aaronballman.com>
wrote:

> On Tue, Mar 24, 2015 at 12:16 PM, David Blaikie <dblaikie at gmail.com>
> wrote:
> >
> >
> > On Tue, Mar 24, 2015 at 8:49 AM, Simon Atanasyan <simon at atanasyan.com>
> > wrote:
> >>
> >> Author: atanasyan
> >> Date: Tue Mar 24 10:49:59 2015
> >> New Revision: 233088
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=233088&view=rev
> >> Log:
> >> [Mips] Suppress "right shift by too large amount" warning
> >>
> >> Visual C++ shows the "right shift by too large amount" warning if
> >> `MipsELFReference` is instantiated for 32-bit target and
> >> `Elf_Rel_Impl::getType`
> >> method has `unsigned char` return type. We can freely suppress the
> warning
> >> in
> >> that case because MIPS 32-bit ABI does not pack multiple relocation
> types
> >> into
> >> the single field `r_type` and the `MipsELFReference::_tag` should be
> >> always
> >> zero in that case.
> >
> >
> > Can we just disable the MSVC warning if it's wrong, then?
>
> IMO, the MSVC warning was correct (the original code was right
> shifting all significant bits away) and should not be disabled.
>

The original code is in a template - it seems reasonable that some
instantiations of the template might legitimately shift all the bits away
(indeed the code change is just suppressing that behavior - the end result
will be the same (if you only have 8 bits, promoting to a 32 bit still
leaves the other bits zero, so shifting down 8 still leaves your result
zero as it would've been with the original code) - so it doesn't seem like
it caught a bug here, or at least this code change didn't fix any bugs)
while other instantiations may not.

- David


>
> ~Aaron
>
> >
> >>
> >>
> >> No functional changes.
> >>
> >> Modified:
> >>     lld/trunk/lib/ReaderWriter/ELF/Mips/MipsELFFile.h
> >>
> >> Modified: lld/trunk/lib/ReaderWriter/ELF/Mips/MipsELFFile.h
> >> URL:
> >>
> http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/ELF/Mips/MipsELFFile.h?rev=233088&r1=233087&r2=233088&view=diff
> >>
> >>
> ==============================================================================
> >> --- lld/trunk/lib/ReaderWriter/ELF/Mips/MipsELFFile.h (original)
> >> +++ lld/trunk/lib/ReaderWriter/ELF/Mips/MipsELFFile.h Tue Mar 24
> 10:49:59
> >> 2015
> >> @@ -101,13 +101,13 @@ public:
> >>        : ELFReference<ELFT>(
> >>              &rel, rel.r_offset - symValue, Reference::KindArch::Mips,
> >>              rel.getType(_isMips64EL) & 0xff,
> rel.getSymbol(_isMips64EL)),
> >> -        _tag(rel.getType(_isMips64EL) >> 8) {}
> >> +        _tag(uint32_t(rel.getType(_isMips64EL)) >> 8) {}
> >>
> >>    MipsELFReference(uint64_t symValue, const Elf_Rel &rel)
> >>        : ELFReference<ELFT>(rel.r_offset - symValue,
> >> Reference::KindArch::Mips,
> >>                             rel.getType(_isMips64EL) & 0xff,
> >>                             rel.getSymbol(_isMips64EL)),
> >> -        _tag(rel.getType(_isMips64EL) >> 8) {}
> >> +        _tag(uint32_t(rel.getType(_isMips64EL)) >> 8) {}
> >>
> >>    uint32_t tag() const override { return _tag; }
> >>    void setTag(uint32_t tag) { _tag = tag; }
> >>
> >>
> >> _______________________________________________
> >> llvm-commits mailing list
> >> llvm-commits at cs.uiuc.edu
> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150324/a3c52ee6/attachment.html>