[PATCH] D22949: Speed up Function::isIntrinsic() by adding a bit to GlobalValue. NFC

[Reid Kleckner via llvm-commits]

rnk added a subscriber: beanz.
rnk added a comment.

In https://reviews.llvm.org/D22949#518862, @jlebar wrote:

> Justin Bogner wrote in an email to the list:
>
> > I'm a bit less comfortable with this change than I was with the other
>
> >  one - it's more of a straight performance hack this way, whereas the
>
> >  other way fit better in a future where target-specific intrinsics are
>
> >  more of a first class idea.
>


Do you think we should move more towards LLVM having a single enum of all target intrinsics, or more towards a place where LLVM has target neutral intrinsics, and then there is a separate target-specific intrinsic ID space that mid-level optimizers can't know about?

This change actually seems like a step towards that world, which the AMDGPU backend is already half-in: all intrinsics are named "llvm.*" and many of them are not included in Intrinsic::ID.

Maybe in the long run we should move all target intrinsics out of include/llvm/IR/Intrinsics*.td, and down into lib/Target. We'd use TargetIntrinsicInfo to look up the ID. Then we can go back to the old, non-bitfield solution. It also makes it easier for us to compile out the intrinsic name to ID tables for disabled targets, which I think @beanz wanted to do. I don't think IR can talk to Target, so we probably can't do this the obvious way.


https://reviews.llvm.org/D22949