fix and test va_arg on f128 on x86

[folkertdev]

tracking issue: #116909

Or well, maybe not fix but clarify. There are no actual behavioral changes, but I do think the new code is more correct.

The clang logic for va_arg is kind of (weirdly) complicated. EmitVAArg looks simple, but the complexity is hiding in the "messing with TypeInfo" bit:

https://github.com/llvm/llvm-project/blob/0e2786dcb5b28f754a1144c27e708d1c42676af9/clang/lib/CodeGen/Targets/X86.cpp#L1080-L1101

RValue X86_32ABIInfo::EmitVAArg(CodeGenFunction &CGF, Address VAListAddr,
                                QualType Ty, AggValueSlot Slot) const {

  auto TypeInfo = getContext().getTypeInfoInChars(Ty);

  CCState State(*const_cast<CGFunctionInfo *>(CGF.CurFnInfo));
  ABIArgInfo AI = classifyArgumentType(Ty, State, /*ArgIndex*/ 0);
  // Empty records are ignored for parameter passing purposes.
  if (AI.isIgnore())
    return Slot.asRValue();

  // x86-32 changes the alignment of certain arguments on the stack.
  //
  // Just messing with TypeInfo like this works because we never pass
  // anything indirectly.
  TypeInfo.Align = CharUnits::fromQuantity(
                getTypeStackAlignInBytes(Ty, TypeInfo.Align.getQuantity()));

  return emitVoidPtrVAArg(CGF, VAListAddr, Ty, /*Indirect*/ false, TypeInfo,
                          CharUnits::fromQuantity(4),
                          /*AllowHigherAlign*/ true, Slot);
}

Relevant for us is that it makes an exception for f128, for which a higher align (16, instead of the slot size of 4) is used.

https://github.com/llvm/llvm-project/blob/0e2786dcb5b28f754a1144c27e708d1c42676af9/clang/lib/CodeGen/Targets/X86.cpp#L575-L583

unsigned X86_32ABIInfo::getTypeStackAlignInBytes(QualType Ty,
                                                 unsigned Align) const {
  // Otherwise, if the alignment is less than or equal to the minimum ABI
  // alignment, just use the default; the backend will handle this.
  if (Align <= MinABIStackAlignInBytes)
    return 0; // Use default alignment.

  if (Ty->isFloat128Type())
    return 16;

Our code does not have this TypeInfo idea, and it's not correct in general to just use AllowHigherAlign::Yes. But using it only for f128 works. We don't need to consider more complicated cases like structs containing f128 fields.

The code here used is_like_windows before. I'm not exactly sure why, the behavior is the same for all accepted types, except now f128. But windows does not have that type, so its (c-variadic) ABI is just an LLVM fabrication. The x86 code does not seem to make a distinction based on the ABI, so having the same behavior across targets seems best to me.

va_arg to fetch an i128 argument is currently broken in clang. i128 on the target is apparently a clang extension anyway, so kind of low priority. f128 I only fixed somewhat recently in llvm/llvm-project#218017.

r? beetrees or @tgross35

(this may cause a small conflict with #163021)

[tgross35]

Is there a way to poke this into being a problem in the runtime tests?

[folkertdev]

Hmm, maybe with i686-pc-windows-gnu? I have no way to run that though. Also I think the c-variadic roundtrip.rs test would actually have caught it, it adds a dummy u32 argument to test the alignment handling.

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.