[lld] r264945 - Remove useless unreachable. Switch coverage already gives us this. NFC

Mehdi Amini via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 30 16:03:32 PDT 2016


> On Mar 30, 2016, at 3:49 PM, David Blaikie <dblaikie at gmail.com> wrote:
> 
> 
> 
> On Wed, Mar 30, 2016 at 3:47 PM, Mehdi Amini via llvm-commits <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>> wrote:
> 
> > On Mar 30, 2016, at 3:34 PM, Pete Cooper via llvm-commits <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>> wrote:
> >
> > Author: pete
> > Date: Wed Mar 30 17:34:37 2016
> > New Revision: 264945
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=264945&view=rev <http://llvm.org/viewvc/llvm-project?rev=264945&view=rev>
> > Log:
> > Remove useless unreachable.  Switch coverage already gives us this.  NFC
> 
> Does the optimizer has a notion of the switch coverage here? Or is the front-end generating an unreachable after your change?
> 
> In GCC, the issue is that they are pessimistic about enum values - they assume (technically correct, of course) that an enum can have any of its representable values, which means that it could contain a value that isn't one of the enumerators & thus the switch could be bypassed entirely and so a return after the switch is needed.
> 
> Clang avoids the possible false positive by being optimistic and assuming that the enum will only contain enumerator values.

Clang could be more aggressive and generate an unreachable in the default-generated block of the switch instruction in this case.

-- 
Mehdi



>  
> Also it is not totally NFC as llvm_unreachable is lowered to a real check in an assert build.
> 
> I think that still qualifies under what I'd consider NFC. An assertion firing isn't functionality, as such - just bugs.
>  
> 
> --
> Mehdi
> 
> 
> 
> >
> > Modified:
> >    lld/trunk/lib/ReaderWriter/MachO/MachONormalizedFileFromAtoms.cpp
> >
> > Modified: lld/trunk/lib/ReaderWriter/MachO/MachONormalizedFileFromAtoms.cpp
> > URL: http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/MachO/MachONormalizedFileFromAtoms.cpp?rev=264945&r1=264944&r2=264945&view=diff <http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/MachO/MachONormalizedFileFromAtoms.cpp?rev=264945&r1=264944&r2=264945&view=diff>
> > ==============================================================================
> > --- lld/trunk/lib/ReaderWriter/MachO/MachONormalizedFileFromAtoms.cpp (original)
> > +++ lld/trunk/lib/ReaderWriter/MachO/MachONormalizedFileFromAtoms.cpp Wed Mar 30 17:34:37 2016
> > @@ -865,7 +865,6 @@ std::error_code Util::getSymbolTableRegi
> >     }
> >     break;
> >   }
> > -  llvm_unreachable("atom->scope() unknown enum value");
> > }
> >
> > std::error_code Util::addSymbols(const lld::File &atomFile,
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits <http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits>
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits <http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160330/ef9f591a/attachment.html>


More information about the llvm-commits mailing list