Differential D109950
[Clang] Enable IC/IF mode for __ibm128 Authored by qiucf on Sep 17 2021, 12:18 AM.
Details
As for 128-bit floating points on PPC, compiler should have three machine modes:
Diff Detail
Event Timeline
Comment Actions I'd appreciate some codegen tests for the new functionality as well (so we're testing not just "do we accept it" but also "does it work" as well), but this looks correct to me. Comment Actions I agree. It doesn't have to be a CodeGen test, just anything that directly verifies that we get the right type, since I think those calls can succeed due to promotion. Comment Actions This has changed the behavior for - // Define __complex128 type corresponding to __float128 (as in GCC headers).
typedef _Complex float __attribute__((mode(TC))) __complex128;
void check() {
// CHECK: alloca { fp128, fp128 } <----- Fails because alloca { x86_fp80, x86_fp80 } is generated instead
__complex128 tmp;
}In TargetInfo.cpp, we used to check long double format to decide whether to return LongDouble or Float128. This patch doesn't, and so the return value has changed from FloatModeKind::Float128 to clang::FloatModeKind::LongDouble (ExplicitType). Should we be checking the format? Comment Actions Oh, yes, I think this should be preserving the old logic there and just adding a new clause for explicit requests for ibm128, right? Comment Actions I think the old logic should be preserved yes. However, I'm not sure if this patch exposes an existing bug. LongDoubleFormat here is llvm::semX87DoubleExtended. In APFloat.cpp, it is defined as - static const fltSemantics semX87DoubleExtended = {16383, -16382, 64, 80};i.e. 80 bit size. Is this the right format for complex type specified using mode 'TC'? I am not very familiar with floating point semantics, so I thought I would ask. Comment Actions The existing logic is that TF and TC should based on long double if the target has a 128-bit long double format, and otherwise they should be based on float128_t if that exists, and otherwise they're invalid. We should definitely not be using long double when it's not a 128-bit format, like if it's the 80-bit x87 format. I do wonder whether that's the right priority between float128_t and a double-double long double. I think that particular configuration only exists on certain PPC targets; does anyone know what GCC does? Comment Actions I don't know what GCC does and am not really sure how to go about figuring it out. I've uploaded a patch preserving old logic for review here - https://reviews.llvm.org/D112975. If I should do something different, please let me know. Thank you for your input! Comment Actions Thanks. @hubert.reinterpretcast, @qiucf, can you verify that other compilers for PPC follow the logic for TF / TC that we now have with Elizabeth's patch? There are three different type specifiers (long double, __ibm128, and float128_t) which represent formally distinct types, and IIUC there are a bunch of different flags and target options that change the meaning of long double and/or disable/enable __ibm128 and float128_t. We care about exactly which type is produced by the mode attribute, so you'll have to do something which is sensitive to the exact formal type, like _Generic or a C++ template or doing a pointer conversion without a cast; checking code generation will only tell us the underlying format. Comment Actions typedef float __attribute__((mode(TF))) __tf128;
typedef float __attribute__((mode(IF))) __if128;
void foo() { printf(__func__); }
void bar() { printf(__func__); }
#define TEST(x) _Generic(x, __ibm128: foo, default: bar)()
int main() {
__if128 x;
__tf128 y;
TEST(x);
TEST(y);
}
// GCC (long double=ieee): foobar
// GCC (long double=ibm): foofoo
// Clang (both): foobarAs discussed in https://reviews.llvm.org/D93377#inline-874356 , GCC and Clang handles them differently: in GCC __ibm128 or __float128 is alias to long double under appropriate option, but Clang sees them as different types. While for explicit mode, Clang thinks float __attribute__((mode(IF))) and __ibm128 are the same type. The patch doesn't change the behavior. For x86 __complex128 check(__complex128 a, __complex128 b) { return a+b; }, GCC also generates two ___addtf3. | ||||||||||||||||||||||
__ibm128 is target-specific, right? So this should return NoFloat if someone requests IF or IC and the target doesn't support it, just like we do when the target doesn't support KF.