[PATCH] D11438: Fix x86_64 fp128 calling convention
David Li via llvm-commits
llvm-commits at lists.llvm.org
Wed Nov 11 00:25:41 PST 2015
davidxl added inline comments.
================
Comment at: lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp:70
@@ +69,3 @@
+ case ISD::BITCAST:
+ case ISD::SELECT:
+ case ISD::SELECT_CC:
----------------
I got curious why these opcodes need to be skipped and picked ISD::SELECT to look at.
Commented line 70 out does trigger error -- but further debugging indicate the bug can be in the original code. In particular, PromoteIntRes_SETCC should call ReplaceValueWith, but does not.
Fixing SETCC handling makes the test case TestMax pass.
Can you verify this is a bug triggered by f128 change?
================
Comment at: lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp:158
@@ +157,3 @@
+ // When LegalInHWReg is true, we take a short-cut in SoftenFloatOperand
+ // to save repeated computations and make the code simpler there.
+ // So here we need to call ReplaceValueWith.
----------------
Where is the short-cut? Is it the new default handling in SoftenFloatOperand? Making short cut there and have weak handshaking seems fragile.
================
Comment at: lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp:752
@@ +751,3 @@
+ // Assume all child nodes were softened if changed in place.
+ if (LegalInHWReg)
+ return false;
----------------
What are the new opcodes that reach here? Please make such opcode explict and add assertions for unexpected ones.
================
Comment at: lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp:791
@@ -711,2 +790,3 @@
- ReplaceValueWith(SDValue(N, 0), Res);
+ if (N != Res.getNode())
+ ReplaceValueWith(SDValue(N, 0), Res);
----------------
This seems redundant -- see above early return.
http://reviews.llvm.org/D11438
More information about the llvm-commits
mailing list