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

[Mehdi Amini via llvm-commits]

> 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.

That's the front-end part right? I was wondering about the optimizer itself. 
I wrote this to test: http://pastebin.com/qK2UCF7z

If you remove the __builtin_unreachable() you don't get the same IR generated by clang, and the optimizer can't assume a range of value for the switch.


>  
> 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.

Fair enough!

-- 
Mehdi



>  
> 
> --
> 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/e55ecff8/attachment.html>