[llvm-commits] [PATCH] PowerPC: More support for Altivec compare operations

[Villmow, Micah]

You can just break at 'default' and you only need 1 llvm_unreachable.

Micah

> -----Original Message-----
> From: Adhemerval Zanella [mailto:azanella at linux.vnet.ibm.com]
> Sent: Wednesday, October 24, 2012 11:46 AM
> To: Villmow, Micah
> Cc: llvm-commits at cs.uiuc.edu
> Subject: Re: [llvm-commits] [PATCH] PowerPC: More support for Altivec
> compare operations
> 
> I thinker the switches a bit and changed to:
> 
> static unsigned int getVCMPInst(MVT::SimpleValueType VecVT,
> ISD::CondCode CC) {
>   switch (CC) {
>     case ISD::SETEQ:
>     case ISD::SETUEQ:
>     case ISD::SETNE:
>     case ISD::SETUNE:
>       if (VecVT == MVT::v16i8)
>         return PPC::VCMPEQUB;
>       else if (VecVT == MVT::v8i16)
>         return PPC::VCMPEQUH;
>       else if (VecVT == MVT::v4i32)
>         return PPC::VCMPEQUW;
>       // v4f32 != v4f32 could be translate to unordered not equal
>       else if (VecVT == MVT::v4f32)
>         return PPC::VCMPEQFP;
>       break;
>     case ISD::SETLT:
>     case ISD::SETGT:
>     case ISD::SETLE:
>     case ISD::SETGE:
>       if (VecVT == MVT::v16i8)
>         return PPC::VCMPGTSB;
>       else if (VecVT == MVT::v8i16)
>         return PPC::VCMPGTSH;
>       else if (VecVT == MVT::v4i32)
>         return PPC::VCMPGTSW;
>       break;
>     case ISD::SETULT:
>     case ISD::SETUGT:
>     case ISD::SETUGE:
>     case ISD::SETULE:
>       if (VecVT == MVT::v16i8)
>         return PPC::VCMPGTUB;
>       else if (VecVT == MVT::v8i16)
>         return PPC::VCMPGTUH;
>       else if (VecVT == MVT::v4i32)
>         return PPC::VCMPGTUW;
>       break;
>     case ISD::SETOEQ:
>       if (VecVT == MVT::v4f32)
>         return PPC::VCMPEQFP;
>       break;
>     case ISD::SETOLT:
>     case ISD::SETOGT:
>     case ISD::SETOLE:
>       if (VecVT == MVT::v4f32)
>         return PPC::VCMPGTFP;
>       break;
>     case ISD::SETOGE:
>       if (VecVT == MVT::v4f32)
>         return PPC::VCMPGEFP;
>       break;
>     default:
>       llvm_unreachable("Invalid integer vector compare condition");
>   }
>   llvm_unreachable("Invalid integer vector compare condition");
> }
> 
> 
> The final 'llvm_unreachable' is needed to avoid compiler warning about
> unused enums.
> 
> 
> On 10/24/2012 03:46 PM, Villmow, Micah wrote:
> > In getVCMPInst, can you put " llvm_unreachable("Invalid vector type
> with conditional code");" at the end of the function and just have the
> switch statements break out instead of duplicating this multiple times.
> > Also can you turn switches with only a single case into a if check.
> >
> >
> > In SelectSetCC, can you factor the common code out of the switch
> statements.
> >> -----Original Message-----
> >> From: llvm-commits-bounces at cs.uiuc.edu [mailto:llvm-commits-
> >> bounces at cs.uiuc.edu] On Behalf Of Adhemerval Zanella
> >> Sent: Wednesday, October 24, 2012 7:50 AM
> >> To: llvm-commits at cs.uiuc.edu
> >> Subject: [llvm-commits] [PATCH] PowerPC: More support for Altivec
> >> compare operations
> >>
> >> My previous patch for altivec compare support was incomplete, it
> didn't
> >> handle the other operators (!=, <, >, etc.). This patch add a better
> >> and more complete support for comparisons for altivec supported
> types
> >> (v16i8, v8i16, v4i32, and v4f32).
> >>
> >> The testcase also covers all the supported comparison operators for
> the
> >> altivec types.
> >>
> >> Any tips, suggestions, comments?
> >
>