<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Dec 23, 2014, at 10:54 AM, Bob Wilson <<a href="mailto:bob.wilson@apple.com" class="">bob.wilson@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><meta http-equiv="Content-Type" content="text/html charset=utf-8" class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div class=""><blockquote type="cite" class=""><div class="">On Dec 23, 2014, at 10:41 AM, Chris Lattner <<a href="mailto:clattner@apple.com" class="">clattner@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><meta http-equiv="Content-Type" content="text/html charset=utf-8" class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">On Dec 23, 2014, at 10:28 AM, Chris Bieneman <<a href="mailto:beanz@apple.com" class="">beanz@apple.com</a>> wrote:<br class=""><div class=""><blockquote type="cite" class=""><div class=""><div style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" class=""><div class=""><div class="" style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div class=""><blockquote type="cite" class=""><div class=""><div class="" style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div class="">It should be straight-forward to have something like LLVMInitializeX86Target/RegisterTargetMachine install the intrinsics into a registry.</div></div></div></blockquote><br class=""></div><div class="">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.</div></div></div></blockquote><br class=""></div><div style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">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.</div></div></blockquote><div class=""><br class=""></div>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.</div></div></div></blockquote><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class=""><br class=""></div><div class="">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.</div></div></div></blockquote><br class=""></div><div class="">I don’t think there’s anything fundamentally difficult, but it’s a big change. For example:</div><div class=""><br class=""></div><div class="">$ git grep Intrinsic:: | wc<br class=""> 3364 12286 281078<br class=""><br class=""></div><div class="">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).</div></div></div></blockquote></div><br class=""><div class="">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:</div><div class=""><br class=""></div><div class=""><div style="margin: 0px; font-size: 10px; font-family: Monaco;" class=""> switch (II->getIntrinsicID()) {</div><div style="font-family: Monaco; font-size: 10px; margin: 0px;" class=""> default: break;</div><div style="margin: 0px; font-size: 10px; font-family: Monaco;" class=""> case Intrinsic::memset:</div><div style="margin: 0px; font-size: 10px; font-family: Monaco;" class=""> case Intrinsic::memcpy:</div><div style="margin: 0px; font-size: 10px; font-family: Monaco;" class=""> case Intrinsic::memmove: {</div></div><div style="margin: 0px; font-size: 10px; font-family: Monaco;" class=""> ...</div><div style="margin: 0px; font-size: 10px; font-family: Monaco;" class=""><div style="margin: 0px;" class=""><div style="margin: 0px;" class=""> case Intrinsic::arm_neon_vld1: {</div><div style="margin: 0px;" class=""> assert(ArgIdx == 0 && "Invalid argument index");</div><div style="margin: 0px;" class=""> assert(Loc.Ptr == II->getArgOperand(ArgIdx) &&</div><div style="margin: 0px;" class=""> "Intrinsic location pointer not argument?");</div><div style="margin: 0px;" class=""> // LLVM's vld1 and vst1 intrinsics currently only support a single</div><div style="margin: 0px;" class=""> // vector register.</div><div style="margin: 0px;" class=""> if (DL)</div><div style="margin: 0px;" class=""> Loc.Size = DL->getTypeStoreSize(II->getType());</div><div style="margin: 0px;" class=""> break;</div><div style="margin: 0px;" class=""> }</div></div></div><div style="margin: 0px; font-size: 10px; font-family: Monaco;" class=""><div style="margin: 0px;" class="">}</div><div style="margin: 0px;" class=""><br class=""></div><div style="margin: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px;" class="">Just change this to:</span></div><div style="margin: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px;" class=""><br class=""></span></div><div style="margin: 0px;" class=""><div style="font-family: Helvetica; font-size: 12px;" class=""><div style="margin: 0px; font-size: 10px; font-family: Monaco;" class=""> switch (II->getIntrinsicID()) {</div><div style="font-family: Monaco; font-size: 10px; margin: 0px;" class=""> case Intrinsic::memset:</div><div style="margin: 0px; font-size: 10px; font-family: Monaco;" class=""> case Intrinsic::memcpy:</div><div style="margin: 0px; font-size: 10px; font-family: Monaco;" class=""> case Intrinsic::memmove: {</div></div><div style="margin: 0px;" class=""> ...</div><div style="margin: 0px;" class=""><div style="margin: 0px;" class=""></div></div></div></div><div class=""><div style="font-family: Monaco; font-size: 10px; margin: 0px;" class=""> default:</div><div style="font-family: Monaco; font-size: 10px; margin: 0px;" class=""> if (II->getName() == “llvm.arm.neon.vld1”) {</div><div style="font-family: Monaco; font-size: 10px; margin: 0px;" class=""> same stuff as above</div><div style="font-family: Monaco; font-size: 10px; margin: 0px;" class=""> }</div><div style="font-family: Monaco; font-size: 10px; margin: 0px;" class=""> break;</div></div><div class=""><br class=""></div><div class="">Spreading ifdefs through the codebase would be really unfortunate.</div><div class=""><br class=""></div><div class="">-Chris</div></body></html>