[cfe-dev] Clang C ABI x64.. bugs?

[Christoffer Lernö via cfe-dev]

Thanks for the answers, I guess my reading of the code wasn’t completely off then.. I was getting a little worried.

Now I’m off trying to figure out in what cases one actually gets the ”only send the top 8 bytes” in x64. (Direct-with-offset), 

Christoffer
AEGIK / www.aegik.se

> On 12 Nov 2020, at 07:18, Craig Topper <craig.topper at gmail.com> wrote:
> 
> Hi Christoffer,
> 
> 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.
> 
> 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.
> 
> For question 3, that looks like it might be a bug.
> 
> ~Craig
> 
> 
> On Wed, Nov 11, 2020 at 3:57 PM Christoffer Lernö via cfe-dev <cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org>> wrote:
> Hi, I tried the Discord to get some answers but got the suggestion to try the mailing list instead.
> 
> 1. I found the following in classification of types for x64:
> 
> > if (const VectorType *VT = Ty->getAs<VectorType>()) {
> >     uint64_t Size = getContext().getTypeSize(VT);
> >     if (Size == 1 || Size == 8 || Size == 16 || Size == 32) {
> >       // gcc passes the following as integer:
> >       // 4 bytes - <4 x char>, <2 x short>, <1 x int>, <1 x float>
> >       // 2 bytes - <2 x char>, <1 x short>
> >       // 1 byte  - <1 x char>
> >       Current = Integer;
> 
> Size 8, 16, 32 seem reasonable, but 1? That seems inconsistent and at odds with the comments.
> 
> 2. Looking at 
> void X86_64ABIInfo::postMerge(unsigned AggregateSize, Class &Lo,
>                               Class &Hi) const {
> 
> There is a comment: "(d) If SSEUP is not preceded by SSE or SSEUP, it is converted to SSE.”
> 
> This is the code:
> 
> > if (Hi == SSEUp && Lo != SSE)
> >     Hi = SSE;
> 
> Seems to be missing a && Lo != SSEUP if that is a possibility (maybe it isn’t?)
> 
> 3. In X86_64ABIInfo::classify
> 
> We have this code for constant arrays:
> 
> >     // Otherwise implement simplified merge. We could be smarter about
> >     // this, but it isn't worth it and would be harder to verify.
> >     Current = NoClass;
> >     uint64_t EltSize = getContext().getTypeSize(AT->getElementType());
> >     uint64_t ArraySize = AT->getSize().getZExtValue();
> > 
> >     // The only case a 256-bit wide vector could be used is when the array
> >     // contains a single 256-bit element. Since Lo and Hi logic isn't extended
> >     // to work for sizes wider than 128, early check and fallback to memory.
> >     //
> >     if (Size > 128 &&
> >         (Size != EltSize || Size > getNativeVectorSizeForAVXABI(AVXLevel)))
> >       return;
> 
> 
> 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”?
> 
> These confused me so I wonder if they are bugs or if I’m misunderstanding what the code does.
> 
> /Christoffer
> 
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org>
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev <https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20201112/8b5aade6/attachment.html>