Let's stop using target specific intrinsics in generic code

There are a few places in llvm's generic codegen that refer to target
specific intrinsics. This is bad layering and we shouldn't do it. It
also means that if we don't build a target we still have to support all
of it's intrinsics and other such annoyances.

The main violator of this is InstCombineCalls - I'd like to push this
into the targets, and just have a case that says "if this is target
specific, call into the target specific library". See below for a patch
that starts to go in that direction by making it easier to tell between
generic and target specific intrinsics.

The other place that this comes up in multiple targets is
AutoUpgrade.cpp, which is kind of special. It probably makes sense to
change these to use intrinsic names instead of IDs - it's probably
overkill to do target specific intrinsic upgrading libraries.

The rest of the issues are mostly x86 (and a bit of arm and aarch64)
specific code that's scattered about. I think these are mostly just
cutting corners instead of doing the right way, but maybe there are
places here where we need to wire in target hooks.

For now I'm considering clang out of scope, but being able to tell which
target an intrinsic is for should also pretty easily clean it up too -
other than a couple of references to ppc.altivec in CGExprScalar and a
strange use of an x86 intrinsic in generic looking EH code, it's all
confined to CGBuiltin.cpp and split up by target anyway.

intrinsics-generic-v-target.patch (12.8 KB)

Yes, yes, a thousand times yes. Please do this.

-Jim

+1, I have been wondering about that for a long time…

So that makes it a 1001 times yes :wink:

From: "Mehdi Amini via llvm-dev" <llvm-dev@lists.llvm.org>
To: "Jim Grosbach" <grosbach@apple.com>
Cc: "llvm-dev" <llvm-dev@lists.llvm.org>
Sent: Thursday, July 14, 2016 5:10:39 PM
Subject: Re: [llvm-dev] Let's stop using target specific intrinsics in generic code

+1, I have been wondering about that for a long time…

So that makes it a 1001 times yes :wink:

+1

Wiring up target hooks might be difficult in ConstantFolding, but I imagine you can just move the logic for the few SSE/SSE2 intrinsics there into InstCombine without any significant problems.

-Hal

There are a few places in llvm's generic codegen that refer to target
specific intrinsics. This is bad layering and we shouldn't do it. It
also means that if we don't build a target we still have to support all
of it's intrinsics and other such annoyances.

The main violator of this is InstCombineCalls - I'd like to push this
into the targets, and just have a case that says "if this is target
specific, call into the target specific library". See below for a patch
that starts to go in that direction by making it easier to tell between
generic and target specific intrinsics.

The other place that this comes up in multiple targets is
AutoUpgrade.cpp, which is kind of special. It probably makes sense to
change these to use intrinsic names instead of IDs - it's probably
overkill to do target specific intrinsic upgrading libraries.

The rest of the issues are mostly x86 (and a bit of arm and aarch64)
specific code that's scattered about. I think these are mostly just
cutting corners instead of doing the right way, but maybe there are
places here where we need to wire in target hooks.

This sounds great! Its been a wart for a while now.

For now I'm considering clang out of scope, but being able to tell which
target an intrinsic is for should also pretty easily clean it up too -
other than a couple of references to ppc.altivec in CGExprScalar and a
strange use of an x86 intrinsic in generic looking EH code, it's all
confined to CGBuiltin.cpp and split up by target anyway.

Aww. Still a better place than before.

+1 :slight_smile: