[LLVMdev] [RFC] Stripping unusable intrinsics

[Bob Wilson]

> On Dec 23, 2014, at 12:41 PM, Chris Lattner <clattner at apple.com> wrote:
> 
> 
>> On Dec 23, 2014, at 10:54 AM, Bob Wilson <bob.wilson at apple.com <mailto:bob.wilson at apple.com>> wrote:
>> 
>> 
>>> On Dec 23, 2014, at 10:41 AM, Chris Lattner <clattner at apple.com <mailto:clattner at apple.com>> wrote:
>>> 
>>> On Dec 23, 2014, at 10:28 AM, Chris Bieneman <beanz at apple.com <mailto:beanz at apple.com>> wrote:
>>>>>> It should be straight-forward to have something like LLVMInitializeX86Target/RegisterTargetMachine install the intrinsics into a registry.
>>>>> 
>>>>> I tried doing that a few years ago. It’s not nearly as easy as it sounds because we’ve got hardcoded references to various target intrinsics scattered throughout the code.
>>>> 
>>>> I was just writing to say exactly this. There are a number of ways we could work toward something like this. I’m completely in favor of a world where Intrinsics are properties of the targets and don’t leach out, however today they do in a lot of places.
>>> 
>>> What are the specific problems here?  Anything that does an equality comparison with the IntrinsicID can be changed to do strcmp with the name.  That would handle the one-off cases like InstCombiner::SimplifyDemandedUseBits in InstCombine.
>>> 
>>> The other cases in InstCombine could be handled similarly, but may be better handled by adding a intrinsic behavior query APIs to the intrinsic registry, or would be better handled (eventually) by adding new attributes to the intrinsics themselves.
>> 
>> I don’t think there’s anything fundamentally difficult, but it’s a big change. For example:
>> 
>> $ git grep Intrinsic:: | wc
>>     3364   12286  281078
>> 
>> The vast majority of those 3,364 lines have hardcoded references to specific intrinsics. Many of them are used in contexts where you can’t easily insert a strcmp (e.g., case values in large switch statements, or worse, the m_Intrinsic PatternMatch templates).
> 
> I don’t find this convincing.  It should be simple to introduce a new m_Intrinsic PatternMatch template that takes a string.  The switches are also straight-forward.  In BasicAA, for example, instead of:
> 
>    switch (II->getIntrinsicID()) {
>     default: break;
>     case Intrinsic::memset:
>     case Intrinsic::memcpy:
>     case Intrinsic::memmove: {
>     ...
>     case Intrinsic::arm_neon_vld1: {
>       assert(ArgIdx == 0 && "Invalid argument index");
>       assert(Loc.Ptr == II->getArgOperand(ArgIdx) &&
>              "Intrinsic location pointer not argument?");
>       // LLVM's vld1 and vst1 intrinsics currently only support a single
>       // vector register.
>       if (DL)
>         Loc.Size = DL->getTypeStoreSize(II->getType());
>       break;
>     }
> }
> 
> Just change this to:
> 
>    switch (II->getIntrinsicID()) {
>     case Intrinsic::memset:
>     case Intrinsic::memcpy:
>     case Intrinsic::memmove: {
>     ...
>     default:
>       if (II->getName() == “llvm.arm.neon.vld1”) {
>         same stuff as above
>       }
>     break;
> 
> Spreading ifdefs through the codebase would be really unfortunate.

I agree. I wasn’t at all trying to discourage anyone from doing as you suggest — just pointing out that it’s not a small project. Much of it would be mechanical, though, and it does seem like the right long-term solution.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20141223/24946b13/attachment.html>