<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<meta name="Generator" content="Microsoft Word 15 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
margin-bottom:.0001pt;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:purple;
text-decoration:underline;}
p.msonormal0, li.msonormal0, div.msonormal0
{mso-style-name:msonormal;
mso-margin-top-alt:auto;
margin-right:0in;
mso-margin-bottom-alt:auto;
margin-left:0in;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
span.EmailStyle18
{mso-style-type:personal;
font-family:"Calibri",sans-serif;
color:windowtext;}
span.EmailStyle20
{mso-style-type:personal-reply;
font-family:"Calibri",sans-serif;
color:windowtext;}
.MsoChpDefault
{mso-style-type:export-only;
font-size:10.0pt;}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang="EN-US" link="blue" vlink="purple">
<div class="WordSection1">
<p class="MsoNormal">It has been pointed out to me that my choice of words could easily be misinterpreted to mean that the people I am responding to do not care about the quality of the code. I’d like to apologize for this.<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">When I say “It seems to me that you don’t actually care about constructing scalable vectors …”, I mean that to complete your work, you do not need to be able to construct scalable vectors. I do not want to imply that you do not care about
the quality of the code, or that this feature works.<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">Thanks,<o:p></o:p></p>
<p class="MsoNormal"> Christopher Tetreault<o:p></o:p></p>
<div>
<div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b>From:</b> llvm-dev <llvm-dev-bounces@lists.llvm.org> <b>On Behalf Of
</b>Chris Tetreault via llvm-dev<br>
<b>Sent:</b> Wednesday, May 13, 2020 12:15 PM<br>
<b>To:</b> James Y Knight <jyknight@google.com><br>
<b>Cc:</b> LLVM Dev <llvm-dev@lists.llvm.org><br>
<b>Subject:</b> [EXT] Re: [llvm-dev] [llvm-commits@lists.llvm.org: Re: [llvm] 2dea3f1 - [SVE] Add new VectorType subclasses]<o:p></o:p></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">The benefit of not having the gap is not having to spend a bunch of time and effort on ABI maintenance. I actually left LLVMVectorTypeKind in at first, and was asked to remove it in code review. Then, I left a gap in the enum where LLVMVectorTypeKind
used to be and was asked in code review to remove the gap. <o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">As for not exposing the names, I think that would be a mistake. The whole point of refactoring vector types is to expose the fact that scalable vectors exist and have to be dealt with. If a ScalableVectorType object is handed to you from
outside of your code, you need to be able to deal with it. Having the ability to correctly handle a scalable vector and having the ability to create one are two separate things. The C api currently has the first ability but not the second. Adding the ability
to construct scalable vectors from C still remains to be implemented. There’s a lot of outstanding work. It all needs to get done, but with limited resources, some tasks will get prioritized over others. I would argue that since support for scalable vectors
is still a work in progress, you should probably not be constructing them and passing them into it.<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">It seems to me that you don’t actually care about constructing scalable vectors (otherwise you’d be asking me to complete the work rather than back it out), so the C api should fully support your use case. You should just update your codebase
to correctly handle scalable vs fixed width vectors. That way, when the day comes that LLVM fully supports scalable vectors, you will be ready.<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal"><b>From:</b> James Y Knight <<a href="mailto:jyknight@google.com">jyknight@google.com</a>>
<br>
<b>Sent:</b> Wednesday, May 13, 2020 11:30 AM<br>
<b>To:</b> Chris Tetreault <<a href="mailto:ctetreau@quicinc.com">ctetreau@quicinc.com</a>><br>
<b>Cc:</b> Joerg Sonnenberger <<a href="mailto:joerg@bec.de">joerg@bec.de</a>>; LLVM Dev <<a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a>><br>
<b>Subject:</b> [EXT] Re: [llvm-dev] [llvm-commits@lists.llvm.org: Re: [llvm] 2dea3f1 - [SVE] Add new VectorType subclasses]<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<p class="MsoNormal">We can't guarantee everything. But, the goal is to not break things in the C API, unless necessary. And, when it is necessary, to make sure it's as obvious as possible of an ABI breakage. E.g., we might delete a function, and create a new
one with a new name, instead of incompatibly changing the number of arguments for an existing function name. Or, might leave a gap in enum values where an item is removed.<o:p></o:p></p>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">For your change, modifying the numeric values of the existing constants is an unnecessary change, and a not-obvious ABI-incompatibility. Just add LLVMScalableVectorTypeKind to the end, instead, and it's fine.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">And, since the C API basically doesn't support scalable vectors yet, and since you aren't modifying the rest of the C API's uses of "VectorType" to FixedVectorType (e.g. the LLVMVectorType function is still named that), I'd suggest simply
leaving the enum value for Fixed vectors named "LLVMVectorTypeKind" as it was before, instead of renaming it to LLVMFixedVectorTypeKind.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<div>
<p class="MsoNormal">On Wed, May 13, 2020 at 1:57 PM Chris Tetreault <<a href="mailto:ctetreau@quicinc.com">ctetreau@quicinc.com</a>> wrote:<o:p></o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0in;margin-bottom:5.0pt">
<div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">Regarding the numerical value of the LLVMTypeKind enum, my understanding is that LLVM-C does not promise to maintain ABI compatability between versions. If I am mistaken, I can
fix this issue. <o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><b>From:</b> llvm-dev <<a href="mailto:llvm-dev-bounces@lists.llvm.org" target="_blank">llvm-dev-bounces@lists.llvm.org</a>>
<b>On Behalf Of </b>James Y Knight via llvm-dev<br>
<b>Sent:</b> Wednesday, May 13, 2020 7:33 AM<br>
<b>To:</b> Joerg Sonnenberger <<a href="mailto:joerg@bec.de" target="_blank">joerg@bec.de</a>>; llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>><br>
<b>Subject:</b> [EXT] Re: [llvm-dev] [<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>: Re: [llvm] 2dea3f1 - [SVE] Add new VectorType subclasses]<o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">Agreed. Those llvm-c changes are wrong, and compatibility needs to be maintained for the numeric values at minimum. Probably also would be a good idea to make LLVMVectorTypeKind
a deprecated alias for LLVMFixedVectorTypeKind.<o:p></o:p></p>
</div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
<div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">On Wed, May 13, 2020 at 9:11 AM Joerg Sonnenberger via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<o:p></o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0in;margin-bottom:5.0pt">
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">Bringing this up on llvm-dev for more general attention.<br>
<br>
The problem here is two fold:<br>
<br>
(1) Reuse of enumeration values is just a major no-go.<br>
(2) I'm not sure why the existing vector types had to be killed<br>
completely.<br>
<br>
But something clearly has to be done here. This majorly affects e.g.<br>
Mesa.<br>
<br>
Joerg<br>
<br>
----- Forwarded message from Joerg Sonnenberger via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> -----<br>
<br>
Date: Sun, 10 May 2020 02:34:12 +0200<br>
From: Joerg Sonnenberger via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>><br>
To: Christopher Tetreault <<a href="mailto:ctetreau@quicinc.com" target="_blank">ctetreau@quicinc.com</a>>, Christopher Tetreault <<a href="mailto:llvmlistbot@llvm.org" target="_blank">llvmlistbot@llvm.org</a>><br>
Cc: <a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
Subject: Re: [llvm] 2dea3f1 - [SVE] Add new VectorType subclasses<br>
Reply-To: Joerg Sonnenberger <<a href="mailto:joerg@bec.de" target="_blank">joerg@bec.de</a>><br>
<br>
On Wed, Apr 22, 2020 at 08:59:20AM -0700, Christopher Tetreault via llvm-commits wrote:<br>
> <br>
> Author: Christopher Tetreault<br>
> Date: 2020-04-22T08:59:01-07:00<br>
> New Revision: 2dea3f129878e929e5d1f00b91a622eb1ec8be4e<br>
> <br>
> URL: <a href="https://github.com/llvm/llvm-project/commit/2dea3f129878e929e5d1f00b91a622eb1ec8be4e" target="_blank">
https://github.com/llvm/llvm-project/commit/2dea3f129878e929e5d1f00b91a622eb1ec8be4e</a><br>
> DIFF: <a href="https://github.com/llvm/llvm-project/commit/2dea3f129878e929e5d1f00b91a622eb1ec8be4e.diff" target="_blank">
https://github.com/llvm/llvm-project/commit/2dea3f129878e929e5d1f00b91a622eb1ec8be4e.diff</a><br>
> <br>
> LOG: [SVE] Add new VectorType subclasses<br>
<br>
This seems to have basically broken both ABI and API of llvm-c. <br>
<br>
Joerg<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
<br>
----- End forwarded message -----<br>
_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><o:p></o:p></p>
</blockquote>
</div>
</div>
</div>
</blockquote>
</div>
</div>
</body>
</html>