[LLVMdev] [RFC] Stripping unusable intrinsics

[Chris Lattner]

> On Dec 23, 2014, at 10:54 AM, Bob Wilson <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.

-Chris
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20141223/eb3be06b/attachment.html>