[PATCH] D43105: [RISCV] Enable __int128_t and uint128_t through clang flag
asb added a comment.
Herald added a subscriber: shiva0217.
As @efriedma noted in https://reviews.llvm.org/D43106, it would be good to have
some test coverage for ABI lowering. I'd suggest updating
test/Driver/riscv32-abi.c with something like:
#ifdef __SIZEOF_INT128__
// CHECK-FORCEINT128-LABEL: define i128 @f_scalar_5(i128 %x)
__int128_t f_scalar_5(__int128_t x) { return x; }
#endif
You could also check the updated defines in in test/Preprocessor/init.c (seems
it should just be the addition of `__SIZEOF_INT128__`).
Repository:
rC Clang
https://reviews.llvm.org/D43105
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43105: [RISCV] Enable __int128_t and uint128_t through clang flag
efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Please post a follow-up to add this to the 7.0 release notes. Repository: rC Clang https://reviews.llvm.org/D43105 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43105: [RISCV] Enable __int128_t and uint128_t through clang flag
mgrang updated this revision to Diff 135112.
mgrang added a comment.
Updated patch to add -fno version of the flag -fforce-enable-int128.
Repository:
rC Clang
https://reviews.llvm.org/D43105
Files:
include/clang/Basic/TargetInfo.h
include/clang/Basic/TargetOptions.h
include/clang/Driver/Options.td
lib/Basic/Targets/Mips.h
lib/Driver/ToolChains/Clang.cpp
lib/Frontend/CompilerInvocation.cpp
test/Driver/types.c
Index: test/Driver/types.c
===
--- /dev/null
+++ test/Driver/types.c
@@ -0,0 +1,18 @@
+// Check whether __int128_t and __uint128_t are supported.
+
+// RUN: not %clang -c --target=riscv32-unknown-linux-gnu -fsyntax-only %s \
+// RUN: 2>&1 | FileCheck %s
+
+// RUN: %clang -c --target=riscv32-unknown-linux-gnu -fsyntax-only %s \
+// RUN: -fno-force-enable-int128 -fforce-enable-int128
+
+// RUN: not %clang -c --target=riscv32-unknown-linux-gnu -fsyntax-only %s \
+// RUN: -fforce-enable-int128 -fno-force-enable-int128
+
+void a() {
+ __int128_t s;
+ __uint128_t t;
+}
+
+// CHECK: error: use of undeclared identifier '__int128_t'
+// CHECK: error: use of undeclared identifier '__uint128_t'
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -2765,6 +2765,7 @@
if (Opts.Triple.empty())
Opts.Triple = llvm::sys::getDefaultTargetTriple();
Opts.OpenCLExtensionsAsWritten = Args.getAllArgValues(OPT_cl_ext_EQ);
+ Opts.ForceEnableInt128 = Args.hasArg(OPT_fforce_enable_int128);
}
bool CompilerInvocation::CreateFromArgs(CompilerInvocation ,
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -4725,6 +4725,12 @@
}
}
+ if (Arg *A = Args.getLastArg(options::OPT_fforce_enable_int128,
+ options::OPT_fno_force_enable_int128)) {
+if (A->getOption().matches(options::OPT_fforce_enable_int128))
+ CmdArgs.push_back("-fforce-enable-int128");
+ }
+
// Finally add the compile command to the compilation.
if (Args.hasArg(options::OPT__SLASH_fallback) &&
Output.getType() == types::TY_Object &&
Index: lib/Basic/Targets/Mips.h
===
--- lib/Basic/Targets/Mips.h
+++ lib/Basic/Targets/Mips.h
@@ -387,7 +387,9 @@
return llvm::makeArrayRef(NewABIRegAliases);
}
- bool hasInt128Type() const override { return ABI == "n32" || ABI == "n64"; }
+ bool hasInt128Type() const override {
+return (ABI == "n32" || ABI == "n64") || getTargetOpts().ForceEnableInt128;
+ }
bool validateTarget(DiagnosticsEngine ) const override;
};
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -839,6 +839,12 @@
def fjump_tables : Flag<["-"], "fjump-tables">, Group;
def fno_jump_tables : Flag<["-"], "fno-jump-tables">, Group, Flags<[CC1Option]>,
HelpText<"Do not use jump tables for lowering switches">;
+def fforce_enable_int128 : Flag<["-"], "fforce-enable-int128">,
+ Group, Flags<[CC1Option]>,
+ HelpText<"Enable support for int128_t type">;
+def fno_force_enable_int128 : Flag<["-"], "fno-force-enable-int128">,
+ Group, Flags<[CC1Option]>,
+ HelpText<"Disable support for int128_t type">;
// Begin sanitizer flags. These should all be core options exposed in all driver
// modes.
Index: include/clang/Basic/TargetOptions.h
===
--- include/clang/Basic/TargetOptions.h
+++ include/clang/Basic/TargetOptions.h
@@ -60,6 +60,9 @@
/// \brief The list of OpenCL extensions to enable or disable, as written on
/// the command line.
std::vector OpenCLExtensionsAsWritten;
+
+ /// \brief If given, enables support for __int128_t and __uint128_t types.
+ bool ForceEnableInt128;
};
} // end namespace clang
Index: include/clang/Basic/TargetInfo.h
===
--- include/clang/Basic/TargetInfo.h
+++ include/clang/Basic/TargetInfo.h
@@ -358,7 +358,7 @@
/// \brief Determine whether the __int128 type is supported on this target.
virtual bool hasInt128Type() const {
-return getPointerWidth(0) >= 64;
+return (getPointerWidth(0) >= 64) || getTargetOpts().ForceEnableInt128;
} // FIXME
/// \brief Determine whether the __float128 type is supported on this target.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43105: [RISCV] Enable __int128_t and uint128_t through clang flag
asb added a comment. In https://reviews.llvm.org/D43105#1003709, @efriedma wrote: > So you want int128_t for compiler-rt itself, so you can use the soft-float > implementation, but you want to make int128_t opt-in to avoid the possibility > of someone getting a link error trying to link code built with clang against > libgcc.a? That seems a little convoluted, but I guess it's okay. That's one particular problem you might encounter if Clang and gcc for RISCV32 made different choices about enabling int128 by default. More generally, we want to ensure the C environment for both GCC and Clang is as close as possible, as any difference like this has the potential for causing difficult to debug differences in behaviour that confuse end users. Repository: rC Clang https://reviews.llvm.org/D43105 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43105: [RISCV] Enable __int128_t and uint128_t through clang flag
efriedma added inline comments. Comment at: include/clang/Driver/Options.td:843 +def fforce_enable_int128 : Flag<["-"], "fforce-enable-int128">, Group, + Flags<[CC1Option]>, HelpText<"Enable support for int128_t type">; We should have an inverse flag -fno-force-enable-int128, like we do for other "-f" flags. (Not sure anyone is actually likely to use it, but better to have it in case someone needs it.) Repository: rC Clang https://reviews.llvm.org/D43105 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43105: [RISCV] Enable __int128_t and uint128_t through clang flag
mgrang updated this revision to Diff 134745.
mgrang edited the summary of this revision.
Repository:
rC Clang
https://reviews.llvm.org/D43105
Files:
include/clang/Basic/TargetInfo.h
include/clang/Basic/TargetOptions.h
include/clang/Driver/Options.td
lib/Basic/Targets/Mips.h
lib/Driver/ToolChains/Clang.cpp
lib/Frontend/CompilerInvocation.cpp
test/Driver/types.c
Index: test/Driver/types.c
===
--- /dev/null
+++ test/Driver/types.c
@@ -0,0 +1,15 @@
+// Check whether __int128_t and __uint128_t are supported.
+
+// RUN: not %clang -c --target=riscv32-unknown-linux-gnu -fsyntax-only %s \
+// RUN: 2>&1 | FileCheck %s
+
+// RUN: %clang -c --target=riscv32-unknown-linux-gnu -fsyntax-only %s \
+// RUN: -fforce-enable-int128
+
+void a() {
+ __int128_t s;
+ __uint128_t t;
+}
+
+// CHECK: error: use of undeclared identifier '__int128_t'
+// CHECK: error: use of undeclared identifier '__uint128_t'
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -2765,6 +2765,7 @@
if (Opts.Triple.empty())
Opts.Triple = llvm::sys::getDefaultTargetTriple();
Opts.OpenCLExtensionsAsWritten = Args.getAllArgValues(OPT_cl_ext_EQ);
+ Opts.ForceEnableInt128 = Args.hasArg(OPT_fforce_enable_int128);
}
bool CompilerInvocation::CreateFromArgs(CompilerInvocation ,
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -4725,6 +4725,9 @@
}
}
+ if (Args.hasArg(options::OPT_fforce_enable_int128))
+CmdArgs.push_back("-fforce-enable-int128");
+
// Finally add the compile command to the compilation.
if (Args.hasArg(options::OPT__SLASH_fallback) &&
Output.getType() == types::TY_Object &&
Index: lib/Basic/Targets/Mips.h
===
--- lib/Basic/Targets/Mips.h
+++ lib/Basic/Targets/Mips.h
@@ -387,7 +387,9 @@
return llvm::makeArrayRef(NewABIRegAliases);
}
- bool hasInt128Type() const override { return ABI == "n32" || ABI == "n64"; }
+ bool hasInt128Type() const override {
+return (ABI == "n32" || ABI == "n64") || getTargetOpts().ForceEnableInt128;
+ }
bool validateTarget(DiagnosticsEngine ) const override;
};
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -839,6 +839,8 @@
def fjump_tables : Flag<["-"], "fjump-tables">, Group;
def fno_jump_tables : Flag<["-"], "fno-jump-tables">, Group, Flags<[CC1Option]>,
HelpText<"Do not use jump tables for lowering switches">;
+def fforce_enable_int128 : Flag<["-"], "fforce-enable-int128">, Group,
+ Flags<[CC1Option]>, HelpText<"Enable support for int128_t type">;
// Begin sanitizer flags. These should all be core options exposed in all driver
// modes.
Index: include/clang/Basic/TargetOptions.h
===
--- include/clang/Basic/TargetOptions.h
+++ include/clang/Basic/TargetOptions.h
@@ -60,6 +60,9 @@
/// \brief The list of OpenCL extensions to enable or disable, as written on
/// the command line.
std::vector OpenCLExtensionsAsWritten;
+
+ /// \brief If given, enables support for __int128_t and __uint128_t types.
+ bool ForceEnableInt128;
};
} // end namespace clang
Index: include/clang/Basic/TargetInfo.h
===
--- include/clang/Basic/TargetInfo.h
+++ include/clang/Basic/TargetInfo.h
@@ -358,7 +358,7 @@
/// \brief Determine whether the __int128 type is supported on this target.
virtual bool hasInt128Type() const {
-return getPointerWidth(0) >= 64;
+return (getPointerWidth(0) >= 64) || getTargetOpts().ForceEnableInt128;
} // FIXME
/// \brief Determine whether the __float128 type is supported on this target.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43105: [RISCV] Enable __int128_t and uint128_t through clang flag
apazos added inline comments.
Comment at: lib/Basic/Targets/RISCV.h:85
+ bool hasInt128Type(const LangOptions ) const override {
+return Opts.UseInt128;
+ }
kito-cheng wrote:
> efriedma wrote:
> > Maybe make this a cross-platform flag, rather than riscv-specific?
> +1, then we can make all other 32 bits target to able easier support float128
> too :)
OK... so we can move the option check to the TargetInfo class, and the
target-specific implementation overrides it as needed.
Repository:
rC Clang
https://reviews.llvm.org/D43105
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43105: [RISCV] Enable __int128_t and uint128_t through clang flag
kito-cheng added inline comments.
Comment at: lib/Basic/Targets/RISCV.h:85
+ bool hasInt128Type(const LangOptions ) const override {
+return Opts.UseInt128;
+ }
efriedma wrote:
> Maybe make this a cross-platform flag, rather than riscv-specific?
+1, then we can make all other 32 bits target to able easier support float128
too :)
Repository:
rC Clang
https://reviews.llvm.org/D43105
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43105: [RISCV] Enable __int128_t and uint128_t through clang flag
kito-cheng added a comment. Hi Eli: > but you want to make int128_t opt-in to avoid the possibility of someone > getting a link error trying to link code built with clang against libgcc.a? Yes, that's the problem we want to avoid, and we actually get the problem if we built libc (newlib) with clang/llvm and used by GCC. Repository: rC Clang https://reviews.llvm.org/D43105 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43105: [RISCV] Enable __int128_t and uint128_t through clang flag
efriedma added a comment. So you want int128_t for compiler-rt itself, so you can use the soft-float implementation, but you want to make int128_t opt-in to avoid the possibility of someone getting a link error trying to link code built with clang against libgcc.a? That seems a little convoluted, but I guess it's okay. Not sure I like the name "-fuse-int128"; doesn't really indicate what it's doing. Maybe "-fforce-enable-int128". Repository: rC Clang https://reviews.llvm.org/D43105 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43105: [RISCV] Enable __int128_t and uint128_t through clang flag
kito-cheng added a comment. Hi Eli: We need that because compiler-rt implement 128 bits soft floating point with int128_t, and RISC-V need that but RV32 doesn't support int128_t, we know it's can be just return true to support that. but we don't want to bring any ABI contemptible issue between GCC and Clang/LLVM. Here is another direction is make GCC support int128_t for RV32, but it's hard to support that in GCC for all 32 bits target[1]. [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60846 Repository: rC Clang https://reviews.llvm.org/D43105 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43105: [RISCV] Enable __int128_t and uint128_t through clang flag
efriedma added a comment.
> This flag can then be used to build compiler-rt for RISCV32.
Can you give a little more context for why this is necessary? As far as I
know, compiler-rt generally supports building for targets which don't have
__int128_t. (If all riscv targets are supposed to support __int128_t, you can
just turn it on without adding a flag.)
Comment at: lib/Basic/Targets/RISCV.h:85
+ bool hasInt128Type(const LangOptions ) const override {
+return Opts.UseInt128;
+ }
Maybe make this a cross-platform flag, rather than riscv-specific?
Comment at: test/Driver/types.c:4
+// RUN: not %clang -c --target=riscv32-unknown-linux-gnu %s \
+// RUN: 2>&1 | FileCheck %s
+
This test won't work unless the riscv backend is enabled; maybe use
-fsyntax-only?
Repository:
rC Clang
https://reviews.llvm.org/D43105
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43105: [RISCV] Enable __int128_t and uint128_t through clang flag
mgrang added a comment. Related compiler-rt patch: https://reviews.llvm.org/D43106 Repository: rC Clang https://reviews.llvm.org/D43105 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43105: [RISCV] Enable __int128_t and uint128_t through clang flag
mgrang created this revision.
mgrang added reviewers: asb, kito-cheng, apazos.
Herald added subscribers: cfe-commits, niosHD, sabuasal, jordy.potman.lists,
simoncook, johnrusso, rbar, aheejin, jgravelle-google, sbc100, sdardis,
dschuff, jfb.
If the flag -fuse-int128 is passed, it will enable support for __int128_t and
uint128_t types.
This flag can then be used to build compiler-rt for RISCV32.
Repository:
rC Clang
https://reviews.llvm.org/D43105
Files:
include/clang/Basic/LangOptions.def
include/clang/Basic/TargetInfo.h
include/clang/Driver/Options.td
lib/Basic/Targets/Mips.h
lib/Basic/Targets/RISCV.h
lib/Basic/Targets/WebAssembly.h
lib/Basic/Targets/X86.h
lib/CodeGen/SwiftCallingConv.cpp
lib/Driver/ToolChains/Clang.cpp
lib/Frontend/CompilerInvocation.cpp
lib/Frontend/InitPreprocessor.cpp
lib/Sema/Sema.cpp
lib/Sema/SemaOverload.cpp
lib/Sema/SemaType.cpp
test/Driver/types.c
Index: test/Driver/types.c
===
--- /dev/null
+++ test/Driver/types.c
@@ -0,0 +1,14 @@
+// Check whether __int128_t and __uint128_t are supported.
+
+// RUN: not %clang -c --target=riscv32-unknown-linux-gnu %s \
+// RUN: 2>&1 | FileCheck %s
+
+// RUN: %clang -c --target=riscv32-unknown-linux-gnu %s -fuse-int128
+
+void a() {
+ __int128_t s;
+ __uint128_t t;
+}
+
+// CHECK: error: use of undeclared identifier '__int128_t'
+// CHECK: error: use of undeclared identifier '__uint128_t'
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -1385,7 +1385,7 @@
break;
}
case DeclSpec::TST_int128:
-if (!S.Context.getTargetInfo().hasInt128Type())
+if (!S.Context.getTargetInfo().hasInt128Type(S.getLangOpts()))
S.Diag(DS.getTypeSpecTypeLoc(), diag::err_type_unsupported)
<< "__int128";
if (DS.getTypeSpecSign() == DeclSpec::TSS_unsigned)
Index: lib/Sema/SemaOverload.cpp
===
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -7680,12 +7680,12 @@
ArithmeticTypes.push_back(S.Context.IntTy);
ArithmeticTypes.push_back(S.Context.LongTy);
ArithmeticTypes.push_back(S.Context.LongLongTy);
-if (S.Context.getTargetInfo().hasInt128Type())
+if (S.Context.getTargetInfo().hasInt128Type(S.getLangOpts()))
ArithmeticTypes.push_back(S.Context.Int128Ty);
ArithmeticTypes.push_back(S.Context.UnsignedIntTy);
ArithmeticTypes.push_back(S.Context.UnsignedLongTy);
ArithmeticTypes.push_back(S.Context.UnsignedLongLongTy);
-if (S.Context.getTargetInfo().hasInt128Type())
+if (S.Context.getTargetInfo().hasInt128Type(S.getLangOpts()))
ArithmeticTypes.push_back(S.Context.UnsignedInt128Ty);
LastPromotedIntegralType = ArithmeticTypes.size();
LastPromotedArithmeticType = ArithmeticTypes.size();
Index: lib/Sema/Sema.cpp
===
--- lib/Sema/Sema.cpp
+++ lib/Sema/Sema.cpp
@@ -196,7 +196,7 @@
return;
// Initialize predefined 128-bit integer types, if needed.
- if (Context.getTargetInfo().hasInt128Type()) {
+ if (Context.getTargetInfo().hasInt128Type(getLangOpts())) {
// If either of the 128-bit integer types are unavailable to name lookup,
// define them now.
DeclarationName Int128 = ("__int128_t");
Index: lib/Frontend/InitPreprocessor.cpp
===
--- lib/Frontend/InitPreprocessor.cpp
+++ lib/Frontend/InitPreprocessor.cpp
@@ -778,7 +778,7 @@
TI.getTypeWidth(TI.getWCharType()), TI, Builder);
DefineTypeSizeof("__SIZEOF_WINT_T__",
TI.getTypeWidth(TI.getWIntType()), TI, Builder);
- if (TI.hasInt128Type())
+ if (TI.hasInt128Type(LangOpts))
DefineTypeSizeof("__SIZEOF_INT128__", 128, TI, Builder);
DefineType("__INTMAX_TYPE__", TI.getIntMaxType(), Builder);
@@ -1048,7 +1048,7 @@
Builder.defineMacro("__IMAGE_SUPPORT__");
}
- if (TI.hasInt128Type() && LangOpts.CPlusPlus && LangOpts.GNUMode) {
+ if (TI.hasInt128Type(LangOpts) && LangOpts.CPlusPlus && LangOpts.GNUMode) {
// For each extended integer type, g++ defines a macro mapping the
// index of the type (0 in this case) in some list of extended types
// to the type.
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -2343,6 +2343,7 @@
Opts.HalfArgsAndReturns = Args.hasArg(OPT_fallow_half_arguments_and_returns)
| Opts.NativeHalfArgsAndReturns;
Opts.GNUAsm = !Args.hasArg(OPT_fno_gnu_inline_asm);
+ Opts.UseInt128 = Args.hasArg(OPT_fuse_int128);
// __declspec is enabled by default for the PS4 by the driver, and also
// enabled for Microsoft Extensions or
