<div dir="ltr">Hi Ana,<div><br></div><div>Thanks for your feedback. I can reproduce this problem and fix it in attached patch. My solution is record both intrinsic name and its prototypes in map. So only a definition with same name and same prototypes will be treated as redefinition. I think this solution is more reasonable and fit more scenarios. I have tested this patch by comparing arm_neon.h, arm_neon.inc before and after this patch. They are all the same. Also I test defining SInst<"vqadd", "sss", "ScSsSiSlSUcSUsSUiSUl">. There is no compile error now.</div>
</div><div class="gmail_extra"><br><br><div class="gmail_quote">2013/8/22 Ana Pazos <span dir="ltr"><<a href="mailto:apazos@codeaurora.org" target="_blank">apazos@codeaurora.org</a>></span><br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div lang="EN-US" link="blue" vlink="purple"><div><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Hi Kevin,<u></u><u></u></span></p><p class="MsoNormal">
<span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">I tried to use your patch for defining the Scalar Arithmetic and and Scalar Reduce Pairwise intrinsics I am working on.<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">I encountered an issue with Scalar Saturating Add which I defined as<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">def SCALAR_QADD   : SInst<"vqadd", "sss", "ScSsSiSlSUcSUsSUiSUl"> <u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">and when enabling tools/clang/lib/Headers/Makefile to build the auto-generated headers arm_neon.h., arm_neon_test.h and arm_neon_sema.h.<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Because I am reusing the intrinsic name “vqadd”, the ARM v7 legacy builtins associated with this same intrinsic name ended up not being added to AArch64 headers as they should.<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Below is one possible solution (simply do not add AArch64 Scalar intrinsics to the AArch64 Map structure; changed <a href="http://arm_neon.td" target="_blank">arm_neon.td</a> and NeonEmitter.cpp).<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">See if you can reproduce the issue, and if you agree with the change below, please add it to your patch.<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">diff --git a/utils/TableGen/NeonEmitter.cpp b/utils/TableGen/NeonEmitter.cpp<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">index 8924278..fece66a 100644<u></u><u></u></span></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">--- a/utils/TableGen/NeonEmitter.cpp<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">+++ b/utils/TableGen/NeonEmitter.cpp<u></u><u></u></span></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">@@ -2396,6 +2396,8 @@ void NeonEmitter::runHeader(raw_ostream &OS) {<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">   std::vector<Record *> RV = Records.getAllDerivedDefinitions("Inst");<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">   // build a map of AArch64 intriniscs to be used in uniqueness checks.<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">+  // The map does not include AArch64 scalar intrinsics because their name<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">+  // mangling and non-overloaded builtins prevent name conflict.<u></u><u></u></span></p><p class="MsoNormal">
<span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">   StringMap<ClassKind> A64IntrinsicMap;<u></u><u></u></span></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">   for (unsigned i = 0, e = RV.size(); i != e; ++i) {<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">     Record *R = RV[i];<u></u><u></u></span></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">@@ -2404,6 +2406,10 @@ void NeonEmitter::runHeader(raw_ostream &OS) {<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">     if (!isA64)<u></u><u></u></span></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">       continue;<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">+       bool isScalar = R->getValueAsBit("isScalar");<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">+    if (isScalar)<u></u><u></u></span></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">+      continue;<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">+<u></u><u></u></span></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">diff --git a/include/clang/Basic/<a href="http://arm_neon.td" target="_blank">arm_neon.td</a> b/include/clang/Basic/<a href="http://arm_neon.td" target="_blank">arm_neon.td</a><u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">index 6918f0a..0a98320 100644<u></u><u></u></span></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">--- a/include/clang/Basic/<a href="http://arm_neon.td" target="_blank">arm_neon.td</a><u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">+++ b/include/clang/Basic/<a href="http://arm_neon.td" target="_blank">arm_neon.td</a><u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">@@ -79,6 +79,7 @@ class Inst <string n, string p, string t, Op o> {<u></u><u></u></span></p><p class="MsoNormal">
<span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">   bit isShift = 0;<u></u><u></u></span></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">   bit isVCVT_N = 0;<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">   bit isA64 = 0;<u></u><u></u></span></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">+  bit isScalar = 0;<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">   // Certain intrinsics have different names than their representative<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">   // instructions. This field allows us to handle this correctly when we<u></u><u></u></span></p><p class="MsoNormal">
<span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">@@ -564,15 +565,46 @@ def SHLL_HIGH_N    : SInst<"vshll_high_n", "ndi", "HcHsHiHUcHUsHUi"<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> // Converting vectors<u></u><u></u></span></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> def VMOVL_HIGH   : SInst<"vmovl_high", "nd", "HcHsHiHUcHUsHUi">;<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">+}<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">+let isA64 = 1, isScalar = 1 in {<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> ////////////////////////////////////////////////////////////////////////////////<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> // Scalar Arithmetic<u></u><u></u></span></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Thanks,<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Ana.<u></u><u></u></span></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Some more background on the issue…<u></u><u></u></span></p>
<div class="im"><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Currently we allow defining AArch64 intrinsics with the same name as ARM intrinsics.<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Usually this happens when the AArch64 intrinsic can handle more types than the ARM intrinsic. It is seen as a superset of ARM intrinsics.<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">So the Prototype string might differ, but they have the same Type string, e.g., "ddd", and Name string.<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Example:<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">def VADD    : IOpInst<"vadd", "ddd", "csilfUcUsUiUlQcQsQiQlQfQUcQUsQUiQUl", OP_ADD>; --> used by ARM NEON<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">...<u></u><u></u></span></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">let isA64 = 1 in {<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">def ADD : IOpInst<"vadd", "ddd", "csilfUcUsUiUlQcQsQiQlQfQUcQUsQUiQUlQd", OP_ADD>; --> with additional Qd type, used by AArch64 NEON<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">...<u></u><u></u></span></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">}<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">When generating the intrinsics definitions and builtin definitions we check for such conflicts using Maps structures.<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">We only include ARM NEON legacy intrinsics and builtin definitions into AArch64 headers/tests if AArch64 does not redefine them. Otherwise the definitions marked as isA64 prevail (they are the superset).<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Now we have a new situation with scalar intrinsics whose Type strings will be different.<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">For example, Scalar Saturating Add, which requires a different Types string "sss".<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">It would be nice to define it as: SInst<"vqadd", "sss", "ScSsSiSlSUcSUsSUiSUl">. But it will fail to compile.<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">There are a couple of possible solutions.<u></u><u></u></span></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">This can be address in your patch or in the next one by whoever needs this fix.<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p></div><p class="MsoNormal"><b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> Kevin Qin [mailto:<a href="mailto:kevinqindev@gmail.com" target="_blank">kevinqindev@gmail.com</a>] <br>
</span></p><div class="im"><b>Sent:</b> Tuesday, August 20, 2013 7:02 PM<br><b>To:</b> Ana Pazos<br></div><b>Cc:</b> Joey Gouly; Jiangning Liu; Hao Liu; <a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a>; <a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><div>
<div class="h5"><br><b>Subject:</b> Re: [PATCH] Aarch64 Neon ACLE scalar instrinsic name mangling with BHSD suffix<u></u><u></u></div></div><p></p><div><div class="h5"><p class="MsoNormal"><u></u> <u></u></p><div><p class="MsoNormal">
Hi Ana,<u></u><u></u></p><div><p class="MsoNormal"><u></u> <u></u></p></div><div><p class="MsoNormal">Sorry for that mistake. I merged and tested my patch on our daily updated internal repo. So it may have latency with truck and just missed Hao's patch at the time I merged my patch. Here is the patch rebased on truck now. Please try again. Thanks.<u></u><u></u></p>
</div></div><div><p class="MsoNormal" style="margin-bottom:12.0pt"><u></u> <u></u></p><div><p class="MsoNormal">2013/8/21 Ana Pazos <<a href="mailto:apazos@codeaurora.org" target="_blank">apazos@codeaurora.org</a>><u></u><u></u></p>
<div><div><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Hi Kevin,</span><u></u><u></u></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">I am trying your patch now.</span><u></u><u></u></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">The first thing I noticed is that the patch does not merge cleanly.</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">It seems you do not have Hao’s previous clang commit (<a href="http://llvm.org/viewvc/llvm-project?view=revision&revision=188452" target="_blank">http://llvm.org/viewvc/llvm-project?view=revision&revision=188452</a> -Clang and AArch64 backend patches to support shll/shl and vmovl instructions and ACLE function).</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Can you rebase and repost your patch?</span><u></u><u></u></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Thanks,</span><u></u><u></u></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Ana.</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> <a href="mailto:cfe-commits-bounces@cs.uiuc.edu" target="_blank">cfe-commits-bounces@cs.uiuc.edu</a> [mailto:<a href="mailto:cfe-commits-bounces@cs.uiuc.edu" target="_blank">cfe-commits-bounces@cs.uiuc.edu</a>] <b>On Behalf Of </b>Kevin Qin<br>
<b>Sent:</b> Monday, August 19, 2013 2:43 AM<br><b>To:</b> Joey Gouly<br><b>Cc:</b> <a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a>; <a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<b>Subject:</b> Re: [PATCH] Aarch64 Neon ACLE scalar instrinsic name mangling with BHSD suffix</span><u></u><u></u></p><div><div><p class="MsoNormal"> <u></u><u></u></p><div><p class="MsoNormal">Hi Joey,<u></u><u></u></p>
<div><p class="MsoNormal"> <u></u><u></u></p></div><div><p class="MsoNormal">   Thanks a lot for your suggestions. I improved my patch as your good advice. I wish to implement some of ACLE intrinsic based on my patch as test, but the full implementation of one ACLE intrinsic need to commit on both llvm and clang. More important thing is,  all scalar ACLE intrinsic are classified to different tables, and these tables are implemented in parallel but none of them is accomplished.  There are complex dependence among intrinsic and may be rewrite frequently.  The main purpose posting this patch at moment is to decrease overlap on such base module and make our parallel development more efficient.<u></u><u></u></p>
</div><div><p class="MsoNormal"> <u></u><u></u></p></div><div><p class="MsoNormal">Best Regards,<u></u><u></u></p></div><div><p class="MsoNormal">Kevin Qin<u></u><u></u></p></div><div><p class="MsoNormal" style="margin-bottom:12.0pt">
 <u></u><u></u></p><div><p class="MsoNormal">2013/8/16 Joey Gouly <<a href="mailto:joey.gouly@arm.com" target="_blank">joey.gouly@arm.com</a>><u></u><u></u></p><p class="MsoNormal">Hi Kevin,<br><br>Clang patches should be sent to cfe-commits. I cc’d them in for you.<br>
<br>You could test this patch by including an ACLE function that you have<br>implemented, that way we can see it works. Or if you are going to submit<br>those soon, maybe it can go in without a test for now. Let’s see what others<br>
think about that.<br><br>Just two minor points.<br><br>> +static std::string Insert_BHSD_Suffix(StringRef typestr){<br><br>This should just return a 'char'.<br><br>> +/// Insert proper 'b' 'h' 's' 'd' behind function name if prefix 'S' is<br>
used.<br>Can you change that to something like<br>  Insert proper 'b', 'h', 's', 'd' suffix if 'S' prefix is used.<br><br>Thanks<br><br>From: <a href="mailto:llvm-commits-bounces@cs.uiuc.edu" target="_blank">llvm-commits-bounces@cs.uiuc.edu</a><br>
[mailto:<a href="mailto:llvm-commits-bounces@cs.uiuc.edu" target="_blank">llvm-commits-bounces@cs.uiuc.edu</a>] On Behalf Of Kevin Qin<br>Sent: 16 August 2013 10:46<br>To: <a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
Subject: [PATCH] Aarch64 Neon ACLE scalar instrinsic name mangling with BHSD<br>suffix<u></u><u></u></p><div><div><p class="MsoNormal" style="margin-bottom:12.0pt"><br>This patch is used to add ‘bhsd’ suffix in Neon ACLE scalar function name.<br>
1. A new type prefix ‘S’ is defined in<br>tools/clang/include/clang/Basic/<a href="http://arm_neon.td" target="_blank">arm_neon.td</a>, which is used to mark whether<br>enable ‘bhsd’ suffix mangle.<br>2. If prefix ‘S’ is found before data type, proper suffix will be added<br>
according below rule:<br>Data Type  Suffix<br>  i8 -> b<br>  i16 -> h<br>  i32 f32 -> s<br>  i64 f64 -> d<br> <br>For example,<br>If we define a new ACLE function in <a href="http://arm_neon.td" target="_blank">arm_neon.td</a> like:<br>
 <br>def FABD : SInst<”vabd”, “sss”,  “ScSsSiSlSfSd”><br> <br>Then, rebuild llvm. We would see below in<br>INSTALL_PATH/lib/clang/3.4/include/arm_neon.h<br> <br>__ai int8_t vabdb_s8(int8_t __a, int8_t __b) {<br>  return (int8_t)__builtin_neon_vabdb_s8(__a, __b); }<br>
__ai int16_t vabdh_s16(int16_t __a, int16_t __b) {<br>  return (int16_t)__builtin_neon_vabdh_s8(__a, __b); }<br>…<br> <br>Proper suffix is inserted in both ACLE intrinsics and builtin functions.<br> <br>Because this patch only works on llvm building time, so I have no idea on<br>
writing test case for this patch. Fortunately, this patch won’t effect on<br>any already defined ACLE functions, for its function depending on the new<br>defined prefix ‘S’. So It is safe for current system and is developed to<br>
help implement new scalar ACLE intrinsics in parallel.<br> <br>You can see each scalar ACLE function corresponds to an unique builtin<br>function. At beginning, I planned to let series of ACLE functions with the<br>same semantics reuse one builtin function. But this would introduce extra<br>
unnecessary convert expression in IR, for different data types have<br>different data width. If I promote all data types to i64 and use single<br>builtin function, then there will be only an i64 version builtin function<br>
existing in IR. Though I can add extra arg in builtin function to record the<br>original data type, but extra data convert IR(sign extension before call and<br>truncation after call) still exists. This will increase the difficulty in<br>
coding lower patterns in backend.<br> <br><br><u></u><u></u></p></div></div></div><p class="MsoNormal"><br><br clear="all"><u></u><u></u></p><div><p class="MsoNormal"> <u></u><u></u></p></div><p class="MsoNormal">-- <u></u><u></u></p>
<div><p class="MsoNormal">Best Regards,<u></u><u></u></p><div><p class="MsoNormal"> <u></u><u></u></p></div><div><p class="MsoNormal">Kevin Qin<u></u><u></u></p></div></div></div></div></div></div></div></div></div><p class="MsoNormal">
<br><br clear="all"><u></u><u></u></p><div><p class="MsoNormal"><u></u> <u></u></p></div><p class="MsoNormal">-- <u></u><u></u></p><div><p class="MsoNormal">Best Regards,<u></u><u></u></p><div><p class="MsoNormal"><u></u> <u></u></p>
</div><div><p class="MsoNormal">Kevin Qin<u></u><u></u></p></div></div></div></div></div></div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div dir="ltr">Best Regards,<div><br></div><div>Kevin Qin</div>
</div>
</div>