[llvm-commits] working on generalized vector intrinsics

[Dan Gohman]

> To me, I agree that having a single type listed for bswap is nice,  
> but I don't think it's worth it to change the return type of ctlz and  
> friends.

For me, the changes to ctlz and friends are more important than bswap,
so I'll take a look at having the bc and .ll readers autoupgrade these.

> Ok.  Is this a theoretical problem or a real problem?  Do you have a  
> target that provides hardware support for element-size ctlz/cttz/ 
> popcount?

Itanium, PowerPC, Alpha, SSE4, and various Cray architectures all have
a ctpop that returns a result of the same size as the operand. This can
mostly be papered over with codegen peeps, but do you want LLVM to
follow what appears to be a consensus among diverse hardware, or follow
the "In C functions should return int because that's how implicit
declaration works" convention? ;-)

> > "The return type must match the argument type."
> 
> Oops, that should be fixed :(

Or the intrinsics themselves should be fixed :).

> ===================================================================
> --- utils/TableGen/CodeGenTarget.cpp	(revision 39829)
> +++ utils/TableGen/CodeGenTarget.cpp	(working copy)
> @@ -93,7 +95,7 @@
>     case MVT::v2f32: return "MVT::v2f32";
>     case MVT::v4f32: return "MVT::v4f32";
>     case MVT::v2f64: return "MVT::v2f64";
> -  case MVT::iPTR:  return "TLI.getPointerTy()";
> +  case MVT::iPTR:  return "MVT::iPTR";
>     default: assert(0 && "ILLEGAL VALUE TYPE!"); return "";
>     }
>   }
> 
> 
> Do you correctly handle this change in all places?

I'm not aware of any places that are wrong at the moment, but I've got
more testing to do in general.

> Does the generated type verifier should check for and reject things  
> like "llvm.cos.i32"?

Yes, because it distinguishes between iAny and fAny, and cos uses fAny.

> Your patch to llvm-upgrade needs improvement :)

:) I'll work on it.

> +  case ISD::CopyFromReg:
> +    Lo = DAG.getNode(ISD::EXTRACT_SUBVECTOR, NewVT, Op,
> +                     DAG.getConstant(0, MVT::i32));
> +    Hi = DAG.getNode(ISD::EXTRACT_SUBVECTOR, NewVT, Op,
> +                     DAG.getConstant(NewNumElts, MVT::i32));
> +    break;
> 
> Why does legalize need to split up CopyFromReg nodes?  Previously,  
> they were always legal, did something change?

That code is used when a CopyFromReg is an operand of a vector intrinsic
that is scalarized. I'm not sure this is the right approach though. I'll
be reevaluating it as I proceed.

Dan

-- 
Dan Gohman, Cray Inc.