[llvm-commits] [llvm] r149808 - /llvm/trunk/lib/Target/X86/X86ISelLowering.cpp

Craig Topper craig.topper at gmail.com
Sun Feb 5 01:22:38 PST 2012


As you can see some of them were asserting and falling through so those
definitely better be unreachable or they would have weird behavior on a
release build. As for the others. It's a bit difficult to quantify
"shouldn't be reachable". And sticking code that attempts to handle
"shouldn't be reachable" after an assert isn't really the best way to deal
with that since the code can never be tested. Clang seems to have had all
of its assert(0) replaced with llvm_unreachable back in September(though
some have creeped in since then). So changing these isn't unprecedented.

Forgot to copy the list the first time.

On Sun, Feb 5, 2012 at 12:54 AM, Duncan Sands <baldrick at free.fr> wrote:

> Hi Craig,
>
> > Convert some assert(0) in default of switch statements to
> llvm_unreachable.
>
> are they really unreachable?  Or are they rather "shouldn't be reachable"?
> My understanding is that llvm_unreachable is for the first case, and assert
> for the second case.
>
> Ciao, Duncan.
>
> >
> > Modified:
> >      llvm/trunk/lib/Target/X86/X86ISelLowering.cpp
> >
> > Modified: llvm/trunk/lib/Target/X86/X86ISelLowering.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86ISelLowering.cpp?rev=149808&r1=149807&r2=149808&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/lib/Target/X86/X86ISelLowering.cpp (original)
> > +++ llvm/trunk/lib/Target/X86/X86ISelLowering.cpp Sat Feb  4 21:43:23
> 2012
> > @@ -4502,9 +4502,7 @@
> >       case X86ISD::MOVSLDUP:
> >       case X86ISD::PALIGN:
> >         return SDValue(); // Not yet implemented.
> > -    default:
> > -      assert(0&&  "unknown target shuffle node");
> > -      return SDValue();
> > +    default: llvm_unreachable("unknown target shuffle node");
> >       }
> >
> >       Index = ShuffleMask[Index];
> > @@ -5813,7 +5811,7 @@
> >     unsigned NewWidth = (NumElems == 4) ? 2 : 4;
> >     EVT NewVT;
> >     switch (VT.getSimpleVT().SimpleTy) {
> > -  default: assert(false&&  "Unexpected!");
> > +  default: llvm_unreachable("Unexpected!");
> >     case MVT::v4f32: NewVT = MVT::v2f64; break;
> >     case MVT::v4i32: NewVT = MVT::v2i64; break;
> >     case MVT::v8i16: NewVT = MVT::v4i32; break;
> > @@ -10577,8 +10575,7 @@
> >     unsigned Reg = 0;
> >     unsigned size = 0;
> >     switch(T.getSimpleVT().SimpleTy) {
> > -  default:
> > -    assert(false&&  "Invalid value type!");
> > +  default: llvm_unreachable("Invalid value type!");
> >     case MVT::i8:  Reg = X86::AL;  size = 1; break;
> >     case MVT::i16: Reg = X86::AX;  size = 2; break;
> >     case MVT::i32: Reg = X86::EAX; size = 4; break;
> > @@ -10696,7 +10693,7 @@
> >     unsigned Opc;
> >     bool ExtraOp = false;
> >     switch (Op.getOpcode()) {
> > -  default: assert(0&&  "Invalid code");
> > +  default: llvm_unreachable("Invalid code");
> >     case ISD::ADDC: Opc = X86ISD::ADD; break;
> >     case ISD::ADDE: Opc = X86ISD::ADC; ExtraOp = true; break;
> >     case ISD::SUBC: Opc = X86ISD::SUB; break;
> > @@ -10838,8 +10835,7 @@
> >     DebugLoc dl = N->getDebugLoc();
> >     switch (N->getOpcode()) {
> >     default:
> > -    assert(false&&  "Do not know how to custom type legalize this
> operation!");
> > -    return;
> > +    llvm_unreachable("Do not know how to custom type legalize this
> operation!");
> >     case ISD::SIGN_EXTEND_INREG:
> >     case ISD::ADDC:
> >     case ISD::ADDE:
> > @@ -12322,7 +12318,7 @@
> >   X86TargetLowering::EmitInstrWithCustomInserter(MachineInstr *MI,
> >                                                  MachineBasicBlock *BB)
> const {
> >     switch (MI->getOpcode()) {
> > -  default: assert(0&&  "Unexpected instr type to insert");
> > +  default: llvm_unreachable("Unexpected instr type to insert");
> >     case X86::TAILJMPd64:
> >     case X86::TAILJMPr64:
> >     case X86::TAILJMPm64:
> > @@ -12699,6 +12695,7 @@
> >       case Intrinsic::x86_avx2_pmovmskb: {
> >         // High bits of movmskp{s|d}, pmovmskb are known zero.
> >         switch (IntId) {
> > +        default: llvm_unreachable("Impossible intrinsic");  // Can't
> reach here.
> >           case Intrinsic::x86_sse_movmsk_ps:      NumLoBits = 4; break;
> >           case Intrinsic::x86_avx_movmsk_ps_256:  NumLoBits = 8; break;
> >           case Intrinsic::x86_sse2_movmsk_pd:     NumLoBits = 2; break;
> >
> >
> > _______________________________________________
> > 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
>



-- 
~Craig
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120205/904a2f21/attachment.html>


More information about the llvm-commits mailing list