<div dir="ltr"><div>Hi Christoffer,</div><div><br></div><div>For question 1, I'm not sure size 1 can occur. I think a vector_size of 1 is going to round up to the nearest byte, but I'm not sure.</div><div dir="ltr"><br></div><div dir="ltr">For question 2, I don't think Lo can ever be SSEUp. We really should extend the Lo/Hi logic to 8 values so we can handle 256 and 512 vectors without hacks. In that case we would have more than SSEUp at a time, but with the Lo/Hi we only have SSEUp in Hi.</div><div dir="ltr"><br></div><div>For question 3, that looks like it might be a bug.</div><div dir="ltr"><br clear="all"><div><div dir="ltr" data-smartmail="gmail_signature">~Craig</div></div><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Nov 11, 2020 at 3:57 PM Christoffer Lernö via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi, I tried the Discord to get some answers but got the suggestion to try the mailing list instead.<br>
<br>
1. I found the following in classification of types for x64:<br>
<br>
> if (const VectorType *VT = Ty->getAs<VectorType>()) {<br>
>     uint64_t Size = getContext().getTypeSize(VT);<br>
>     if (Size == 1 || Size == 8 || Size == 16 || Size == 32) {<br>
>       // gcc passes the following as integer:<br>
>       // 4 bytes - <4 x char>, <2 x short>, <1 x int>, <1 x float><br>
>       // 2 bytes - <2 x char>, <1 x short><br>
>       // 1 byte  - <1 x char><br>
>       Current = Integer;<br>
<br>
Size 8, 16, 32 seem reasonable, but 1? That seems inconsistent and at odds with the comments.<br>
<br>
2. Looking at <br>
void X86_64ABIInfo::postMerge(unsigned AggregateSize, Class &Lo,<br>
                              Class &Hi) const {<br>
<br>
There is a comment: "(d) If SSEUP is not preceded by SSE or SSEUP, it is converted to SSE.”<br>
<br>
This is the code:<br>
<br>
> if (Hi == SSEUp && Lo != SSE)<br>
>     Hi = SSE;<br>
<br>
Seems to be missing a && Lo != SSEUP if that is a possibility (maybe it isn’t?)<br>
<br>
3. In X86_64ABIInfo::classify<br>
<br>
We have this code for constant arrays:<br>
<br>
>     // Otherwise implement simplified merge. We could be smarter about<br>
>     // this, but it isn't worth it and would be harder to verify.<br>
>     Current = NoClass;<br>
>     uint64_t EltSize = getContext().getTypeSize(AT->getElementType());<br>
>     uint64_t ArraySize = AT->getSize().getZExtValue();<br>
> <br>
>     // The only case a 256-bit wide vector could be used is when the array<br>
>     // contains a single 256-bit element. Since Lo and Hi logic isn't extended<br>
>     // to work for sizes wider than 128, early check and fallback to memory.<br>
>     //<br>
>     if (Size > 128 &&<br>
>         (Size != EltSize || Size > getNativeVectorSizeForAVXABI(AVXLevel)))<br>
>       return;<br>
<br>
<br>
The return here is a bit suspicious together with the comment. It says ”fallback to memory” – which seems to assume that Current is set to Memory (happens earlier in the method), however it seems to overlook that we just set Current to ”NoClass” so the fallback would be ”NoClass”?<br>
<br>
These confused me so I wonder if they are bugs or if I’m misunderstanding what the code does.<br>
<br>
/Christoffer<br>
<br>
_______________________________________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br>
</blockquote></div></div>