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

[David Blaikie via llvm-commits]

On Wed, Mar 30, 2016 at 4:02 PM, Mehdi Amini <mehdi.amini at apple.com> wrote:

>
> 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> wrote:
>
>>
>> > On Mar 30, 2016, at 3:34 PM, Pete Cooper via llvm-commits <
>> 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
>> > 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.
>

For a void-returning function I imagine that's true - try it with a
non-void return type. I would expect to see the same behavior with or
without the unreachable in that case.


>
>
>
>
>> 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
>> >
>> ==============================================================================
>> > --- 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
>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> 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/61153da5/attachment.html>