LLVM and System V ABI Inconsistency Question

Hi LLVM Community,

I’m hoping for some insight to a question I stumbled on – I’ve been trying various social networks and contacts to no avail! I was recently looking at the System V ABI Document for X86 and comparing to the LLVM source code, and I had a few questions about register allocation. Basically, when we are doing the post merge step in classifying, say, a struct, the first rule says that we check for either lo or hi to be in memory, and then set the whole thing to memory. But in the implementation, it only checks hi (and then changes lo). I thought if it’s not an oversight, it might be the case that lo can never be set to memory, or something like that? I’m hoping that you have some insight because I’m not sure if this is an oversight on my part! I shared some screenshots here:

https://twitter.com/vsoch/status/1435687187979268099

And the link to the code in question is here:

https://github.com/llvm/llvm-project/blob/main/clang/lib/CodeGen/TargetInfo.cpp#L2726

Thank you so much for your help! :blush:

Best,

Vanessa

Hi Vanessa,

I think the two users of the final classification, X86_64ABIInfo::classifyReturnType and X86_64ABIInfo::classifyArgumentType, don’t look at Hi if Lo == Memory. They both return from their respective “switch (Lo)”. So I guess the code took a shortcut and didn’t bother updating Hi.

Hi Craig,

Thanks for your speedy response! What about line 3080 where we check if Lo == Memory, and then do the post-merge providing both?

https://github.com/llvm/llvm-project/blob/f68939d3d91c3e1b57fba5450fa9146c3dcf5fdc/clang/lib/CodeGen/TargetInfo.cpp#L3080

I’m new to this code base so I don’t have a case in mind that would lead to an error (and hopefully if there was an issue it would have been caught by now) but the fact that the spec (and the function docstring) make the statement and then it’s missing in the implementation makes me uneasy at best. I was hoping that whomever wrote this might add a comment to explain (without a doubt) why it was okay to not follow the spec, or if that’s not possible, to update the implementation to match the spec. What are your thoughts?

Best,

Vanessa