[PATCH] D105762: [X86] Teach X86FloatingPoint's handleCall to only erase the FP stack if there is a regmask operand that clobbers the FP stack.

[Craig Topper via Phabricator via llvm-commits]

craig.topper added a comment.

In D105762#2869378 <https://reviews.llvm.org/D105762#2869378>, @pengfei wrote:

>> __alloca only updates EAX and ESP so this isn't wrong.
>
> "is" or "isn't"?

"isn't wrong" was correct, but I've rewritten to hopefully be more readable



================
Comment at: llvm/lib/Target/X86/X86FloatingPoint.cpp:988-989
     MachineOperand &Op = MI.getOperand(i);
+    // Check if this call clobbers the FP stack.
+    // is sufficient.
+    if (Op.isRegMask()) {
----------------
pengfei wrote:
> I found we don't clobber FP stack by default. E.g. `void foo(void)`. The interesting thing is even if we passing registers to/from foo, e.g. `long double foo(void)` or `long double foo(long double)`, the regmasks of FP are still 0.
> Did I misunderstand something here?
The regmask bits indicate which registers aren't clobbered. So a 0 means clobbered.


================
Comment at: llvm/lib/Target/X86/X86FloatingPoint.cpp:995
+      for (unsigned i = 1; i != 8; ++i)
+        assert(Op.clobbersPhysReg(X86::FP0 + i) == ClobbersFP0 &&
+               "Inconsistent FP register clobber");
----------------
pengfei wrote:
> Why can't the callee only clobber one or a few registers?
The later code that calls popReg until the stack is empty would need to be updated in a non-trivial way to handle that.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105762/new/

https://reviews.llvm.org/D105762