<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=us-ascii">
<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-top:0in;
margin-right:0in;
margin-bottom:8.0pt;
margin-left:0in;
line-height:106%;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:#0563C1;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:#954F72;
text-decoration:underline;}
p.MsoListParagraph, li.MsoListParagraph, div.MsoListParagraph
{mso-style-priority:34;
margin-top:0in;
margin-right:0in;
margin-bottom:8.0pt;
margin-left:.5in;
mso-add-space:auto;
line-height:106%;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
p.MsoListParagraphCxSpFirst, li.MsoListParagraphCxSpFirst, div.MsoListParagraphCxSpFirst
{mso-style-priority:34;
mso-style-type:export-only;
margin-top:0in;
margin-right:0in;
margin-bottom:0in;
margin-left:.5in;
margin-bottom:.0001pt;
mso-add-space:auto;
line-height:106%;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
p.MsoListParagraphCxSpMiddle, li.MsoListParagraphCxSpMiddle, div.MsoListParagraphCxSpMiddle
{mso-style-priority:34;
mso-style-type:export-only;
margin-top:0in;
margin-right:0in;
margin-bottom:0in;
margin-left:.5in;
margin-bottom:.0001pt;
mso-add-space:auto;
line-height:106%;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
p.MsoListParagraphCxSpLast, li.MsoListParagraphCxSpLast, div.MsoListParagraphCxSpLast
{mso-style-priority:34;
mso-style-type:export-only;
margin-top:0in;
margin-right:0in;
margin-bottom:8.0pt;
margin-left:.5in;
mso-add-space:auto;
line-height:106%;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
span.EmailStyle17
{mso-style-type:personal-compose;
font-family:"Calibri",sans-serif;
color:windowtext;}
.MsoChpDefault
{mso-style-type:export-only;
font-family:"Calibri",sans-serif;}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
/* List Definitions */
@list l0
{mso-list-id:2067752029;
mso-list-type:hybrid;
mso-list-template-ids:442897782 67698703 67698713 67698715 67698703 67698713 67698715 67698703 67698713 67698715;}
@list l0:level1
{mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;}
@list l0:level2
{mso-level-number-format:alpha-lower;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;}
@list l0:level3
{mso-level-number-format:roman-lower;
mso-level-tab-stop:none;
mso-level-number-position:right;
text-indent:-9.0pt;}
@list l0:level4
{mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;}
@list l0:level5
{mso-level-number-format:alpha-lower;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;}
@list l0:level6
{mso-level-number-format:roman-lower;
mso-level-tab-stop:none;
mso-level-number-position:right;
text-indent:-9.0pt;}
@list l0:level7
{mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;}
@list l0:level8
{mso-level-number-format:alpha-lower;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;}
@list l0:level9
{mso-level-number-format:roman-lower;
mso-level-tab-stop:none;
mso-level-number-position:right;
text-indent:-9.0pt;}
ol
{margin-bottom:0in;}
ul
{margin-bottom:0in;}
--></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="#0563C1" vlink="#954F72">
<div class="WordSection1">
<p class="MsoNormal">Hi,<o:p></o:p></p>
<p class="MsoNormal"> I am helping with the effort to implement scalable vectors in the codebase in order to add support for generating SVE code in the Arm backend. I would like to propose a refactor of the
<span style="font-family:"Courier New"">Type</span> class hierarchy in order to eliminate issues related to the misuse of
<span style="font-family:"Courier New"">SequentialType::getNumElements()</span>.<b>
</b>I would like to introduce a new class <span style="font-family:"Courier New"">
FixedVectorType</span> that inherits from <span style="font-family:"Courier New"">
SequentialType</span> and <span style="font-family:"Courier New"">VectorType</span>.
<span style="font-family:"Courier New"">VectorType</span> would no longer inherit from
<span style="font-family:"Courier New"">SequentialType,</span> instead directly inheriting from<span style="font-family:"Courier New""> Type</span>. After this change, it will be statically impossible to accidentally call
<span style="font-family:"Courier New"">SequentialType::getNumElements()</span> via a
<span style="font-family:"Courier New"">VectorType</span> pointer.<b><o:p></o:p></b></p>
<p class="MsoNormal"><b>Background:<o:p></o:p></b></p>
<p class="MsoNormal"> Recently, scalable vectors have been introduced into the codebase. Previously, vectors have been written
<span style="font-family:"Courier New""><n x ty></span> in IR, where <span style="font-family:"Courier New"">
n</span> is a fixed number of elements known at compile time, and <span style="font-family:"Courier New"">
ty</span> is some type. Scalable vectors are written <span style="font-family:"Courier New"">
<vscale x n x ty> </span>where <span style="font-family:"Courier New"">vscale</span> is a runtime constant value. A new function has been added to
<span style="font-family:"Courier New"">VectorType</span> (defined in llvm/IR/DerivedTypes.h),
<span style="font-family:"Courier New"">getElementCount()</span>, that returns an
<span style="font-family:"Courier New"">ElementCount</span>, which is defined as such in llvm/Support/TypeSize.h:<o:p></o:p></p>
<p class="MsoNormal" style="margin-bottom:0in;margin-bottom:.0001pt;line-height:normal">
<span style="font-family:"Courier New"">class ElementCount {<o:p></o:p></span></p>
<p class="MsoNormal" style="margin-bottom:0in;margin-bottom:.0001pt;text-indent:.5in;line-height:normal">
<span style="font-family:"Courier New"">public:<o:p></o:p></span></p>
<p class="MsoNormal" style="margin-bottom:0in;margin-bottom:.0001pt;text-indent:.5in;line-height:normal">
<span style="font-family:"Courier New""> unsigned Min;<o:p></o:p></span></p>
<p class="MsoNormal" style="margin-bottom:0in;margin-bottom:.0001pt;text-indent:.5in;line-height:normal">
<span style="font-family:"Courier New""> bool Scalable;<o:p></o:p></span></p>
<p class="MsoNormal" style="margin-bottom:0in;margin-bottom:.0001pt;text-indent:.5in;line-height:normal">
<span style="font-family:"Courier New""> …<o:p></o:p></span></p>
<p class="MsoNormal" style="text-indent:.5in;line-height:normal"><span style="font-family:"Courier New"">}<o:p></o:p></span></p>
<p class="MsoNormal" style="line-height:normal"> <span style="font-family:"Courier New"">
Min</span> is the minimum number of elements in the vector (the “<span style="font-family:"Courier New"">n</span>” in
<span style="font-family:"Courier New""><vscale x n x ty></span>), and <span style="font-family:"Courier New"">
Scalable</span> is true if the vector is scalable (<span style="font-family:"Courier New"">true</span> for
<span style="font-family:"Courier New""><vscale x n x ty></span>, <span style="font-family:"Courier New"">
false</span> for <span style="font-family:"Courier New""><n x ty></span>) The idea is that if a vector is not scalable, then
<span style="font-family:"Courier New"">Min</span> is exactly equal to the number of vector elements, but if the vector is scalable, then the number of vector elements is equal to some runtime-constant multiple of
<span style="font-family:"Courier New"">Min</span>. The key takeaway here is that scalable vectors and fixed length vectors need to be treated differently by the compiler. For a fixed length vector, it is valid to iterate over the vector elements, but this
is impossible for a scalable vector.<o:p></o:p></p>
<p class="MsoNormal" style="line-height:normal"><b>Discussion:</b><o:p></o:p></p>
<p class="MsoNormal" style="text-indent:.5in;line-height:normal">The trouble is that all instances of
<span style="font-family:"Courier New"">VectorType</span> have <span style="font-family:"Courier New"">
getNumElements()</span> inherited from <span style="font-family:"Courier New"">SequentialType</span>. Prior to the introduction of scalable vectors, this function was guaranteed to return the number of elements in a vector or array. Today, there is a comment
that documents the fact that this returns only the minimum number of elements for scalable vectors, however there exists a ton of code in the codebase that is now misusing
<span style="font-family:"Courier New"">getNumElements()</span>. Some examples:<o:p></o:p></p>
<p class="MsoNormal" style="line-height:normal"> <span style="font-family:"Courier New"">
Auto *V = VectorType::get(Ty, SomeOtherVec->getNumElements());<o:p></o:p></span></p>
<p class="MsoNormal" style="line-height:normal"> This code was previously perfectly fine but is incorrect for scalable vectors. When scalable vectors were introduced
<span style="font-family:"Courier New"">VectorType::get()</span> was refactored to take a bool to tell if the vector is scalable. This bool has a default value of
<span style="font-family:"Courier New"">false</span>. In this example, <span style="font-family:"Courier New"">
get()</span> is returning a non-scalable vector even if <span style="font-family:"Courier New"">
SomeOtherVec</span> was scalable. This will manifest later in some unrelated code as a type mismatch between a scalable and fixed length vector.<o:p></o:p></p>
<p class="MsoNormal" style="line-height:normal"> <span style="font-family:"Courier New"">
for (unsigned I = 0; I < SomeVec->getNumElements(); ++I) { … }</span><o:p></o:p></p>
<p class="MsoNormal" style="line-height:normal"> Previously, since there was no notion of scalable vectors, this was perfectly reasonable code. However, for scalable vectors, this is always a bug.<o:p></o:p></p>
<p class="MsoNormal" style="line-height:normal"> With vigilance in code review, and good test coverage we will eventually find and squash most of these bugs. Unfortunately, code review is hard, and test coverage isn’t perfect. Bugs will continue
to slip through as long as it’s easier to do the wrong thing. <o:p></o:p></p>
<p class="MsoNormal" style="line-height:normal"> One other factor to consider, is that there is a great deal of code which deals exclusively with fixed length vectors. Any backend for which there are no scalable vectors should not need to care
about their existence. Even in Arm, if Neon code is being generated, then the vectors will never be scalable. In this code, the current status quo is perfectly fine, and any code related to checking if the vector is scalable is just noise.<o:p></o:p></p>
<p class="MsoNormal" style="line-height:normal"><b>Proposal:<o:p></o:p></b></p>
<p class="MsoNormal" style="line-height:normal"> In order to support users who only need fixed width vectors, and to ensure that nobody can accidentally call
<span style="font-family:"Courier New"">getNumElements()</span> on a scalable vector, I am proposing the introduction of a new
<span style="font-family:"Courier New"">FixedVectorType</span> which inherits from both
<span style="font-family:"Courier New"">VectorType</span> and <span style="font-family:"Courier New"">
SequentialType</span>. In turn, <span style="font-family:"Courier New"">VectorType</span> will no longer inherit from
<span style="font-family:"Courier New"">SequentialType</span>. An example of what this will look like, with some misc. functions omitted for clarity:<o:p></o:p></p>
<p class="MsoNormal" style="margin-bottom:0in;margin-bottom:.0001pt;line-height:normal">
<span style="font-family:"Courier New"">class VectorType : public Type {<o:p></o:p></span></p>
<p class="MsoNormal" style="margin-bottom:0in;margin-bottom:.0001pt;line-height:normal">
<span style="font-family:"Courier New"">public:<o:p></o:p></span></p>
<p class="MsoNormal" style="margin-bottom:0in;margin-bottom:.0001pt;line-height:normal">
<span style="font-family:"Courier New""> static VectorType *get(Type *ElementType, ElementCount EC);<o:p></o:p></span></p>
<p class="MsoNormal" style="margin-bottom:0in;margin-bottom:.0001pt;line-height:normal">
<span style="font-family:"Courier New""><o:p> </o:p></span></p>
<p class="MsoNormal" style="margin-bottom:0in;margin-bottom:.0001pt;line-height:normal">
<span style="font-family:"Courier New""> Type *getElementType() const;<o:p></o:p></span></p>
<p class="MsoNormal" style="margin-bottom:0in;margin-bottom:.0001pt;line-height:normal">
<span style="font-family:"Courier New""> ElementCount getElementCount() const;<o:p></o:p></span></p>
<p class="MsoNormal" style="margin-bottom:0in;margin-bottom:.0001pt;line-height:normal">
<span style="font-family:"Courier New""> bool isScalable() const;<o:p></o:p></span></p>
<p class="MsoNormal" style="margin-bottom:0in;margin-bottom:.0001pt;line-height:normal">
<span style="font-family:"Courier New"">};<o:p></o:p></span></p>
<p class="MsoNormal" style="margin-bottom:0in;margin-bottom:.0001pt;line-height:normal">
<span style="font-family:"Courier New""><o:p> </o:p></span></p>
<p class="MsoNormal" style="margin-bottom:0in;margin-bottom:.0001pt;line-height:normal">
<span style="font-family:"Courier New"">class FixedVectorType : public VectorType, public SequentialType {<o:p></o:p></span></p>
<p class="MsoNormal" style="margin-bottom:0in;margin-bottom:.0001pt;line-height:normal">
<span style="font-family:"Courier New"">public:<o:p></o:p></span></p>
<p class="MsoNormal" style="margin-bottom:0in;margin-bottom:.0001pt;line-height:normal">
<span style="font-family:"Courier New""> static FixedVectorType *get(Type *ElementType, unsigned NumElts);<o:p></o:p></span></p>
<p class="MsoNormal" style="margin-bottom:0in;margin-bottom:.0001pt;line-height:normal">
<span style="font-family:"Courier New"">};<o:p></o:p></span></p>
<p class="MsoNormal" style="margin-bottom:0in;margin-bottom:.0001pt;line-height:normal">
<span style="font-family:"Courier New""><o:p> </o:p></span></p>
<p class="MsoNormal" style="margin-bottom:0in;margin-bottom:.0001pt;line-height:normal">
<span style="font-family:"Courier New"">class SequentialType : public CompositeType {<o:p></o:p></span></p>
<p class="MsoNormal" style="margin-bottom:0in;margin-bottom:.0001pt;line-height:normal">
<span style="font-family:"Courier New"">public:<o:p></o:p></span></p>
<p class="MsoNormal" style="margin-bottom:0in;margin-bottom:.0001pt;line-height:normal">
<span style="font-family:"Courier New""> uint64_t getNumElements() const { return NumElements; }<o:p></o:p></span></p>
<p class="MsoNormal" style="line-height:normal"><span style="font-family:"Courier New"">};<o:p></o:p></span></p>
<p class="MsoNormal" style="line-height:normal"> In this proposed architecture,
<span style="font-family:"Courier New"">VectorType</span> does not have a <span style="font-family:"Courier New"">
getNumElements()</span> function because it does not inherit from <span style="font-family:"Courier New"">
SequentialType</span>. In generic code, users will call <span style="font-family:"Courier New"">
VectorType::get()</span> to obtain a new instance of <span style="font-family:"Courier New"">
VectorType</span> just as they always have. <span style="font-family:"Courier New"">
VectorType</span> implements the safe subset of functionality of fixed and scalable vectors that is suitable for use anywhere. If the user passes
<span style="font-family:"Courier New"">false</span> to the scalable parameter of
<span style="font-family:"Courier New"">get()</span>, they will get an instance of
<span style="font-family:"Courier New"">FixedVectorType</span> back. Users can then inspect its type and cast it to
<span style="font-family:"Courier New"">FixedVectorType</span> using the usual mechanisms. In code that deals exclusively in fixed length vectors, the user can call
<span style="font-family:"Courier New"">FixedVectorType::get()</span> to directly get an instance of
<span style="font-family:"Courier New"">FixedVectorType</span>, and their code can remain largely unchanged from how it was prior to the introduction of scalable vectors. At this time, there exists no use case that is only valid for scalable vectors, so no
<span style="font-family:"Courier New"">ScalableVectorType</span> is being added.<o:p></o:p></p>
<p class="MsoNormal" style="line-height:normal"> With this change, in generic code it is now impossible to accidentally call
<span style="font-family:"Courier New"">getNumElements()</span> on a scalable vector. If a user tries to pass a scalable vector to a function that expects a fixed length vector, they will encounter a compilation failure at the site of the bug, rather than a
runtime error in some unrelated code. If a user attempts to cast a scalable vector to
<span style="font-family:"Courier New"">FixedVectorType</span>, the cast will fail at the call site. This will make it easier to track down all the places that are currently incorrect, and will prevent future developers from introducing bugs by misusing
<span style="font-family:"Courier New"">getNumElements()</span>.<o:p></o:p></p>
<p class="MsoNormal" style="line-height:normal"><b>Outstanding design choice:<o:p></o:p></b></p>
<p class="MsoNormal" style="line-height:normal"> One issue with this architecture as proposed is the fact that
<span style="font-family:"Courier New"">SequentialType</span> (by way of <span style="font-family:"Courier New"">
CompositeType</span>) inherits from <span style="font-family:"Courier New"">Type</span>. This introduces a diamond inheritance in
<span style="font-family:"Courier New"">FixedVectorType</span>. Unfortunately, <span style="font-family:"Courier New"">
llvm::cast</span> uses a c-style cast internally, so we cannot use virtual inheritance to resolve this issue. Thus, we have a few options:<o:p></o:p></p>
<ol style="margin-top:0in" start="1" type="1">
<li class="MsoListParagraphCxSpFirst" style="margin-left:0in;mso-add-space:auto;line-height:normal;mso-list:l0 level1 lfo1">
Break <span style="font-family:"Courier New"">CompositeType’s</span> inheritance on
<span style="font-family:"Courier New"">Type</span> and introduce functions to convert from a
<span style="font-family:"Courier New"">Type</span> to a <span style="font-family:"Courier New"">
CompositeType</span> and vice versa. The conversion from <span style="font-family:"Courier New"">
CompositeType</span> is always safe because all instances of <span style="font-family:"Courier New"">
CompositeType</span> (<span style="font-family:"Courier New"">StructType</span>, <span style="font-family:"Courier New"">
ArrayType</span>, and <span style="font-family:"Courier New"">FixedVectorType</span>) are instances of
<span style="font-family:"Courier New"">Type</span>. A <span style="font-family:"Courier New"">
CompositeType</span> can be cast to the most derived class, then back to <span style="font-family:"Courier New"">
Type</span>. The other way is not always safe, so a function will need to be added to check if a
<span style="font-family:"Courier New"">Type</span> is an instance of <span style="font-family:"Courier New"">
CompositeType</span>. This change is not that big, and I have a prototype implementation up at
<a href="https://reviews.llvm.org/D75486">https://reviews.llvm.org/D75486</a> (<b><i>[SVE] Make CompositeType not inherit from Type)</i></b><o:p></o:p>
<ol style="margin-top:0in" start="1" type="a">
<li class="MsoListParagraphCxSpMiddle" style="margin-left:0in;mso-add-space:auto;line-height:normal;mso-list:l0 level2 lfo1">
Pros: this approach would result in minimal changes to the codebase. If the llvm casts can be made to work for the conversion functions, then it would touch very few files.<o:p></o:p></li><li class="MsoListParagraphCxSpMiddle" style="margin-left:0in;mso-add-space:auto;line-height:normal;mso-list:l0 level2 lfo1">
Cons: There are those who think that <span style="font-family:"Courier New"">CompositeType</span> adds little value and should be removed. Now would be an ideal time to do this. Additionally, the conversion functions would be more complicated if we left
<span style="font-family:"Courier New"">CompositeType</span> in.<o:p></o:p></li></ol>
</li><li class="MsoListParagraphCxSpMiddle" style="margin-left:0in;mso-add-space:auto;line-height:normal;mso-list:l0 level1 lfo1">
Remove <span style="font-family:"Courier New"">CompositeType</span> and break <span style="font-family:"Courier New"">
SequentialType’s</span> inheritance of <span style="font-family:"Courier New"">Type</span>. Add functions to convert a
<span style="font-family:"Courier New"">SequentialType</span> to and from <span style="font-family:"Courier New"">
Type</span>. The conversion functions would work the same as those in option 1 above. Currently, there exists only one class that derives directly from
<span style="font-family:"Courier New"">CompositeType</span>: <span style="font-family:"Courier New"">
StructType</span>. The functionality of <span style="font-family:"Courier New"">CompositeType</span> can be directly moved into
<span style="font-family:"Courier New"">StructType</span>, and APIs that use <span style="font-family:"Courier New"">
CompositeType</span> can directly use <span style="font-family:"Courier New"">Type</span> and cast appropriately. We feel that this would be a fairly simple change, and we have a prototype implementation up at
<a href="https://reviews.llvm.org/D75660">https://reviews.llvm.org/D75660</a> (<b><i>Remove CompositeType class</i></b>)<o:p></o:p>
<ol style="margin-top:0in" start="1" type="a">
<li class="MsoListParagraphCxSpMiddle" style="margin-left:0in;mso-add-space:auto;line-height:normal;mso-list:l0 level2 lfo1">
Pros: Removing <span style="font-family:"Courier New"">CompositeType</span> would simplify the type hierarchy. Leaving
<span style="font-family:"Courier New"">SequentialType</span> in would simplify some code and be more typesafe than having a
<span style="font-family:"Courier New"">getSequentialNumElements</span> on <span style="font-family:"Courier New"">
Type</span>.<o:p></o:p></li><li class="MsoListParagraphCxSpMiddle" style="margin-left:0in;mso-add-space:auto;line-height:normal;mso-list:l0 level2 lfo1">
Cons: The value of <span style="font-family:"Courier New"">SequentialType</span> has also been called into question. If we wanted to remove it, now would be a good time. Conversion functions add complexity to the design. Introduces additional casting from
<span style="font-family:"Courier New"">Type</span>.<o:p></o:p></li></ol>
</li><li class="MsoListParagraphCxSpLast" style="margin-left:0in;mso-add-space:auto;line-height:normal;mso-list:l0 level1 lfo1">
Remove <span style="font-family:"Courier New"">CompositeType</span> and <span style="font-family:"Courier New"">
SequentialType</span>. Roll the functions directly into the most derived classes. A helper function can be added to
<span style="font-family:"Courier New"">Type</span> to handle choosing from <span style="font-family:"Courier New"">
FixedVectorType</span> and <span style="font-family:"Courier New"">ArrayType</span> and calling
<span style="font-family:"Courier New"">getNumElements()</span>:<o:p></o:p></li></ol>
<p class="MsoNormal" style="mso-margin-top-alt:0in;margin-right:0in;margin-bottom:0in;margin-left:.5in;margin-bottom:.0001pt;line-height:normal">
<span style="font-family:"Courier New"">static unsigned getSequentialNumElements() {<o:p></o:p></span></p>
<p class="MsoNormal" style="mso-margin-top-alt:0in;margin-right:0in;margin-bottom:0in;margin-left:.5in;margin-bottom:.0001pt;line-height:normal">
<span style="font-family:"Courier New""> assert(isSequentialType()); // This already exists and does the<o:p></o:p></span></p>
<p class="MsoNormal" style="mso-margin-top-alt:0in;margin-right:0in;margin-bottom:0in;margin-left:.5in;margin-bottom:.0001pt;line-height:normal">
<span style="font-family:"Courier New""> // right thing<o:p></o:p></span></p>
<p class="MsoNormal" style="mso-margin-top-alt:0in;margin-right:0in;margin-bottom:0in;margin-left:.5in;margin-bottom:.0001pt;line-height:normal">
<span style="font-family:"Courier New""> if (auto *AT = dyn_cast<ArrayType>(this))<o:p></o:p></span></p>
<p class="MsoNormal" style="mso-margin-top-alt:0in;margin-right:0in;margin-bottom:0in;margin-left:.5in;margin-bottom:.0001pt;line-height:normal">
<span style="font-family:"Courier New""> return AT->getNumElements();<o:p></o:p></span></p>
<p class="MsoNormal" style="mso-margin-top-alt:0in;margin-right:0in;margin-bottom:0in;margin-left:.5in;margin-bottom:.0001pt;line-height:normal">
<span style="font-family:"Courier New""> return cast<FixedVectorType>(this)->getNumElements();<o:p></o:p></span></p>
<p class="MsoNormal" style="margin-left:.5in;line-height:normal"><span style="font-family:"Courier New"">}<o:p></o:p></span></p>
<p class="MsoNormal" style="margin-left:.5in;line-height:normal">A prototype implementation of this strategy can be found at
<a href="https://reviews.llvm.org/D75661">https://reviews.llvm.org/D75661</a> (<b><i>Remove SequentialType from the type heirarchy.</i></b>)<o:p></o:p></p>
<ol style="margin-top:0in" start="3" type="1">
<ol style="margin-top:0in" start="1" type="a">
<li class="MsoListParagraphCxSpFirst" style="margin-left:0in;mso-add-space:auto;line-height:normal;mso-list:l0 level2 lfo1">
Pros: By removing the multiple inheritance completely, we greatly simplify the design and eliminate the need for any conversion functions. The value of
<span style="font-family:"Courier New"">CompositeType</span> and <span style="font-family:"Courier New"">
SequentialType</span> has been called into question, and removing them now might be of benefit to the codebase<o:p></o:p></li><li class="MsoListParagraphCxSpLast" style="margin-left:0in;mso-add-space:auto;line-height:normal;mso-list:l0 level2 lfo1">
Cons: <span style="font-family:"Courier New"">getSequentialNumElements()</span> has similar issues to those that we are trying to solve in the first place and potentially subverts the whole design. Omitting
<span style="font-family:"Courier New"">getSequentialNumElements()</span> would add lots of code duplication. Introduces additional casting from
<span style="font-family:"Courier New"">Type</span>.<o:p></o:p></li></ol>
</ol>
<p class="MsoNormal" style="text-indent:.5in;line-height:normal">I believe that all three of these options are reasonable. My personal preference is currently option 2. I think that option 3’s
<span style="font-family:"Courier New"">getSequentialNumElements()</span> subverts the design because every
<span style="font-family:"Courier New"">Type</span> has <span style="font-family:"Courier New"">
getSequentialNumElements()</span>, it is tempting to just call it. However, the cast will fail at the call site in debug, and in release it will return a garbage value rather than a value that works most of the time. For option 1, the existence of
<span style="font-family:"Courier New"">CompositeType</span> complicates the conversion logic for little benefit.<o:p></o:p></p>
<p class="MsoNormal" style="line-height:normal"><b>Conclusion:<o:p></o:p></b></p>
<p class="MsoNormal" style="line-height:normal"> Thank you for your time in reviewing this RFC. Your feedback on my work is greatly appreciated.<o:p></o:p></p>
<p class="MsoNormal" style="line-height:normal"><o:p> </o:p></p>
<p class="MsoNormal" style="line-height:normal">Thank you,<o:p></o:p></p>
<p class="MsoNormal" style="line-height:normal"> Christopher Tetreault
<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
</body>
</html>