llvm dev - Jul 2016 - 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.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: intrinsics-generic-v-target.patch
Type: text/x-patch
Size: 13086 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160714/2c444164/attachment.bin>

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

-Jim
> On Jul 14, 2016, at 2:03 PM, Justin Bogner via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
> 
> 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>_______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

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

So that makes it a 1001 times yes ;)

— 
Mehdi
> On Jul 14, 2016, at 3:08 PM, Jim Grosbach via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
> 
> Yes, yes, a thousand times yes. Please do this.
> 
> -Jim
> 
>> On Jul 14, 2016, at 2:03 PM, Justin Bogner via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
>> 
>> 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>_______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
> 
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

On Thu, Jul 14, 2016 at 2:03 PM, Justin Bogner via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> 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.

>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
>
-- 
Saleem Abdulrasool
compnerd (at) compnerd (dot) org
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160714/e516998d/attachment.html>

+1 :)

On Thu, Jul 14, 2016, 2:03 PM Justin Bogner via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> 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.
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160715/3a2ce8ce/attachment.html>