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

[Craig Topper via cfe-dev]

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> 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
> 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/20201111/7eb95fd5/attachment.html>