freaknbigpanda wrote:

Hi @rjmccall the psABI doesn't say anything about these  "empty" types AFAIK, 
they are language constructs that don't map to anything in the ABI.

I did a refactor to make the intent more clear that we are ignoring empty CXX 
record bases and fields everywhere, all of the tests still pass with no changes 
after this. Basically the classification algorithm already ignored empty fields 
- they got classified as no class (in most cases at least), it was only these 
vector classification cases that got screwed up due to the empty bases & fields.

I agree that 
https://github.com/llvm/llvm-project/commit/e8a486ea97895a18e1bba75431d37d9758886084
 is a bit messy, basically what it is doing is implementing logic that looks at 
the base classes for a CXX record and overwrites the computed low high 
classifications with MEMORY if the record is a struct that doesn't contain just 
a single legal vector field.

For example for the following struct: 

template<int id>
struct XMM1 { 
    UInt64x2 x;
};
struct XMM2 : XMM1<0>, XMM1<1> {
};

The original classification algorithm would have gone through each of the two 
bases classified each as SSE SSEUP, and then calculated XMM2 to also be SSE 
SSEUP. This is wrong, the psABI clearly states that unless SSE is followed by 
only SSEUP the whole thing should be passed in memory. So this is what the 
change above was trying to fix.

I think what we should be doing is classifying each eightbyte of structs 
separately and then merging them all at the end, instead of trying to do 
everything recursively in 2 8byte/low high increments. I really want to just 
rewrite the entire classification algorith, there are too many weird corner 
case bugs and the recursion makes it too hard for people to follow.

https://github.com/llvm/llvm-project/pull/187814
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to