[PATCH] D11438: Fix x86_64 fp128 calling convention
Chih-hung Hsieh via llvm-commits
llvm-commits at lists.llvm.org
Fri Oct 16 16:54:28 PDT 2015
Somehow a few previous emails were not seen athttp://reviews.llvm.org/D11438
Does anyone know the cause of that?
As promised in my previous email, I gave more thought about changing
getCommonSubClass instead of InstrEmitter.cpp. The problem was that
getCommonSubClass returns FR128 register class as the common sub class
with VR128. But FR128 does not contain v16i8 and 128-bit vector types.
I tried to add 128-bit vector types into FR128. That caused many other
errors that I doubt is the right direction.
Hence, it seems to me that my best solution now is to take care of
getCommonSubClass inside EmitCopyFromReg of InstrEmitter.cpp,
but not to change other users of getCommonSubClass.
The other users do not look like having clear requirement of
something like ComRC->hasType(VT).
The best alternative I have to my current patch is shown bellow.
(1) Almost no change to InstrEmitter.cpp, which just passes the VT.SimpleTy
to getCommonSubClass, which should then guarantee that returned
ComRC will have type VT.
(2) Change getCommonSubclass to take one more parameter with default
value. So other callers do not need any change. When the extra
parameter is not MVT::SimpleValueType::Any, returns found
common sub class only if it contains the given SimpleValueType.
(3) getCommonSubclass calls firstCommonClass but it's not the only caller.
So change firstCommonClass similarly.
Well, this is actually more change than my current patch.
So, I am still in favor of my current patch.
Index: lib/CodeGen/SelectionDAG/InstrEmitter.cpp
===================================================================
--- lib/CodeGen/SelectionDAG/InstrEmitter.cpp (revision 247905)
+++ lib/CodeGen/SelectionDAG/InstrEmitter.cpp (working copy)
@@ -139,7 +139,7 @@
UseRC = RC;
else if (RC) {
const TargetRegisterClass *ComRC =
- TRI->getCommonSubClass(UseRC, RC);
+ TRI->getCommonSubClass(UseRC, RC, VT.SimpleTy);
// If multiple uses expect disjoint register classes, we emit
// copies in AddRegisterOperand.
if (ComRC)
Index: include/llvm/Target/TargetRegisterInfo.h
===================================================================
--- include/llvm/Target/TargetRegisterInfo.h (revision 247905)
+++ include/llvm/Target/TargetRegisterInfo.h (working copy)
@@ -620,10 +620,13 @@
}
/// getCommonSubClass - find the largest common subclass of A and B. Return
- /// NULL if there is no common subclass.
+ /// NULL if there is no common subclass. The common subclass should contain
+ /// simple value type SVT if it is not the Any type.
const TargetRegisterClass *
getCommonSubClass(const TargetRegisterClass *A,
- const TargetRegisterClass *B) const;
+ const TargetRegisterClass *B,
+ const MVT::SimpleValueType SVT =
+ MVT::SimpleValueType::Any) const;
/// getPointerRegClass - Returns a TargetRegisterClass used for pointer
/// values. If a target supports multiple different pointer
register classes,
Index: lib/CodeGen/TargetRegisterInfo.cpp
===================================================================
--- lib/CodeGen/TargetRegisterInfo.cpp (revision 247905)
+++ lib/CodeGen/TargetRegisterInfo.cpp (working copy)
@@ -166,16 +166,24 @@
static inline
const TargetRegisterClass *firstCommonClass(const uint32_t *A,
const uint32_t *B,
- const TargetRegisterInfo *TRI) {
+ const TargetRegisterInfo *TRI,
+ const MVT::SimpleValueType SVT =
+ MVT::SimpleValueType::Any) {
+ const MVT VT(SVT);
for (unsigned I = 0, E = TRI->getNumRegClasses(); I < E; I += 32)
- if (unsigned Common = *A++ & *B++)
- return TRI->getRegClass(I + countTrailingZeros(Common));
+ if (unsigned Common = *A++ & *B++) {
+ const TargetRegisterClass *RC =
+ TRI->getRegClass(I + countTrailingZeros(Common));
+ if (SVT == MVT::SimpleValueType::Any || RC->hasType(VT))
+ return RC;
+ }
return nullptr;
}
const TargetRegisterClass *
TargetRegisterInfo::getCommonSubClass(const TargetRegisterClass *A,
- const TargetRegisterClass *B) const {
+ const TargetRegisterClass *B,
+ const MVT::SimpleValueType SVT) const {
// First take care of the trivial cases.
if (A == B)
return A;
@@ -184,7 +192,7 @@
// Register classes are ordered topologically, so the largest common
// sub-class it the common sub-class with the smallest ID.
- return firstCommonClass(A->getSubClassMask(), B->getSubClassMask(), this);
+ return firstCommonClass(A->getSubClassMask(), B->getSubClassMask(),
this, SVT);
}
const TargetRegisterClass *
On Tue, Oct 13, 2015 at 4:51 PM, Chih-hung Hsieh <chh at google.com> wrote:
> On Tue, Oct 13, 2015 at 9:21 AM, Quentin Colombet <qcolombet at apple.com>
> wrote:
>
>>
>> This is interesting observation: fp128 gets split into 2-64-bit registers.
>> Looks like we could go incrementally here.
>> What about:
>> 1. Changing the android ABI to pass the fp128 via xmm registers, but let
>> the values live wherever they currently live in the meantime.
>> 2. Fixing the AsmPrinter.
>> 3. Fixing the split of the value into 2-64bit registers.
>>
>>
> 2. is probably the simplest and independent part that can go in before the
> others. The change is only in lib/Target/X86/X86MCInstLower.cpp. If it is
> left out, llvm can compile libm, but abort on -g in some cases.
>
> 1. was what I wanted at the beginning and I don't mind using 2 64-bit
> registers in other operations if that is possible.
>
> However, after I made the following changes for calling convention:
> lib/Target/X86/X86RegisterInfo.td
> lib/Target/X86/X86CallingConv.td
> lib/Target/X86/X86InstrInfo.td
> I found missing instructions to handle fp128 values load/store in xmm
> registers, although there were many to process vectors in xmm.
> So we need some extra load/store instructions in
> lib/Target/X86/X86InstrSSE.td
> Once f128 is 'legal' in register, it seemed easier for llvm to pass it
> around than converting back-and-forth between 128-bit and 2x64-bit
> registers.
> In fact since almost all f128 operations are handled by library functions,
> the values are mostly passed around and in f128 registers.
>
> I didn't try it but I think it would probably require more twist of
> current llvm code to keep f128 in 2 64-bit registers and pass in xmm.
> Anyway, if anyone knows how to configure llvm easily, we can try that.
>
> Actually most of the changes were required to compile libm, which uses
> unions to access floating point bits as integers. We need instructions to
> handle i128 vs f128 issues. Hence, the need of only "partial" logic in
> LegalizeFloatTypes.cpp.
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151016/72462a19/attachment.html>
More information about the llvm-commits
mailing list