george.burgess.iv updated this revision to Diff 220416. george.burgess.iv marked 4 inline comments as done. george.burgess.iv added a reviewer: efriedma. george.burgess.iv added a comment.
Chatted with Eli offline; updated here to reflect the conclusions of that. Importantly, this patch readds some of the peepholes we try to not diagnose, since the target users of this quite commonly do things that, after macro expansion, fold into e.g., `(int)(3.0 + 1)`. By wrapping these into `ConstantExpr`s at the cast point, we get our nice guaranteed lowering to 0 FP/vector ops in IR. Similarly for struct assignment, I couldn't find a way to get an assignment of a struct of multiple fields to turn into a not-memcpy, so it seems safe to me to keep that around. I have tests to this effect, and am happy to add more if people can think of cases these tests may not adequately cover. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D38479/new/ https://reviews.llvm.org/D38479 Files: clang/docs/UsersManual.rst clang/include/clang/AST/Expr.h clang/include/clang/Basic/DiagnosticSemaKinds.td clang/include/clang/Basic/LangOptions.def clang/include/clang/Driver/CC1Options.td clang/include/clang/Sema/Sema.h clang/lib/AST/Expr.cpp clang/lib/Driver/ToolChains/Arch/AArch64.cpp clang/lib/Driver/ToolChains/Clang.cpp clang/lib/Frontend/CompilerInvocation.cpp clang/lib/Sema/Sema.cpp clang/lib/Sema/SemaCast.cpp clang/lib/Sema/SemaDecl.cpp clang/lib/Sema/SemaExprCXX.cpp clang/test/CodeGen/aarch64-mgeneral_regs_only.c clang/test/Sema/aarch64-mgeneral_regs_only.c clang/test/SemaCXX/aarch64-mgeneral_regs_only.cpp
Index: clang/test/SemaCXX/aarch64-mgeneral_regs_only.cpp =================================================================== --- /dev/null +++ clang/test/SemaCXX/aarch64-mgeneral_regs_only.cpp @@ -0,0 +1,214 @@ +// RUN: %clang_cc1 -triple aarch64-linux-eabi -std=c++11 -general-regs-only %s -verify -DBANNED=float -Wno-unused-value +// RUN: %clang_cc1 -triple aarch64-linux-eabi -std=c++11 -general-regs-only %s -verify -DBANNED=int '-DVECATTR=__attribute__((ext_vector_type(2)))' -Wno-unused-value +// RUN: %clang_cc1 -triple aarch64-linux-eabi -std=c++11 -general-regs-only %s -verify -DBANNED=FloatTypedef -Wno-unused-value + +using FloatTypedef = float; + +#ifndef VECATTR +#define VECATTR +#define BannedToInt(x) (x) +#else +#define BannedToInt(x) ((x)[0]) +#endif + +typedef BANNED BannedTy VECATTR; + +namespace default_args { +int foo(BannedTy = 0); // expected-error 2{{use of floating-point or vector values is disabled}} +int bar(int = 1.0); + +void baz(int a = foo()); // expected-note{{from use of default argument here}} +void bazz(int a = bar()); + +void test() { + foo(); // expected-note{{from use of default argument here}} + bar(); + baz(); // expected-note{{from use of default argument here}} + baz(4); + bazz(); +} +} // namespace default_args + +namespace conversions { +struct ConvertToFloat { + explicit operator BannedTy(); +}; +struct ConvertToFloatImplicit { /* implicit */ + operator BannedTy(); +}; +struct ConvertFromFloat { + ConvertFromFloat(BannedTy); +}; + +void takeFloat(BannedTy); +void takeConvertFromFloat(ConvertFromFloat); +void takeConvertFromFloatRef(const ConvertFromFloat &); + +void test() { + BannedTy(ConvertToFloat()); + + takeFloat(ConvertToFloatImplicit{}); // expected-error{{use of floating-point or vector values is disabled}} + + ConvertFromFloat(0); // expected-error{{use of floating-point or vector values is disabled}} + ConvertFromFloat(ConvertToFloatImplicit{}); // expected-error{{use of floating-point or vector values is disabled}} + + ConvertFromFloat{0}; // expected-error{{use of floating-point or vector values is disabled}} + ConvertFromFloat{ConvertToFloatImplicit{}}; // expected-error{{use of floating-point or vector values is disabled}} + + takeConvertFromFloat(0); // expected-error{{use of floating-point or vector values is disabled}} + takeConvertFromFloatRef(0); // expected-error{{use of floating-point or vector values is disabled}} + + takeConvertFromFloat(ConvertFromFloat(0)); // expected-error{{use of floating-point or vector values is disabled}} + takeConvertFromFloatRef(ConvertFromFloat(0)); // expected-error{{use of floating-point or vector values is disabled}} +} + +void takeImplicitFloat(BannedTy = ConvertToFloatImplicit()); // expected-error{{use of floating-point or vector values is disabled}} +void test2() { + takeImplicitFloat(); // expected-note{{from use of default argument here}} +} +} // namespace conversions + +namespace refs { +struct BannedRef { + const BannedTy &f; + BannedRef(const BannedTy &f) : f(f) {} +}; + +BannedTy &getBanned(); +BannedTy getBannedVal(); + +void foo() { + BannedTy &a = getBanned(); + BannedTy b = getBanned(); // expected-error{{use of floating-point or vector values is disabled}} + const BannedTy &c = getBanned(); + const BannedTy &d = getBannedVal(); // expected-error{{use of floating-point or vector values is disabled}} + + const int &e = 1.0; + const int &f = BannedToInt(getBannedVal()); // expected-error{{use of floating-point or vector values is disabled}} + + BannedRef{getBanned()}; + BannedRef{getBannedVal()}; // expected-error{{use of floating-point or vector values is disabled}} +} +} // namespace refs + +// The struct assignment special casing we do isn't really intended for C++ at +// the moment. +namespace struct_assign { +struct Foo { + BannedTy f; + int i; + + Foo(const BannedTy &f, int i); + + Foo() = default; + Foo(const Foo &) = default; + Foo(Foo &&) = default; + + Foo &operator=(const Foo &) = default; // expected-error{{use of floating-point or vector values is disabled}} + Foo &operator=(Foo &&) = default; // expected-error 2{{use of floating-point or vector values is disabled}} +}; + +void foo(Foo &f) { + Foo a = f; // expected-error{{use of floating-point or vector values is disabled}} + Foo b = static_cast<Foo &&>(f); // expected-error{{use of floating-point or vector values is disabled}} + Foo c = Foo{}; // expected-error{{use of floating-point or vector values is disabled}} + Foo d = Foo{1, 1}; // expected-error{{use of floating-point or vector values is disabled}} + + Foo *p = &f; + *p = f; // expected-note{{in implicit copy assignment operator for}} + *p = // expected-note{{in implicit move assignment operator for}} + static_cast<Foo &&>(f); // expected-error{{use of floating-point or vector values is disabled}} +} + +struct Bar { // expected-error 3{{use of floating-point or vector values is disabled}} + BannedTy f; + int i; +}; + +void bar(Bar &r) { + Bar a = r; // expected-error{{use of floating-point or vector values is disabled}} + Bar b = static_cast<Bar &&>(r); // expected-error{{use of floating-point or vector values is disabled}} + + Bar *p = &r; + *p = a; // expected-note{{in implicit copy assignment operator for}} + *p = // expected-note{{in implicit move assignment operator for}} + static_cast<Bar &&>(b); // expected-error{{use of floating-point or vector values is disabled}} +} +} + +namespace class_init { +struct Foo { + float f = 1.0; // expected-error{{use of floating-point or vector values is disabled}} + int i = 1.0; + float j; + + Foo() : j(1) // expected-error{{use of floating-point or vector values is disabled}} + {} +}; +} // namespace class_init + +namespace templates { +float bar(); + +template <typename T> +T foo(int t = bar()) { // expected-error 2{{use of floating-point or vector values is disabled}} + return t; // expected-error{{use of floating-point or vector values is disabled}} +} + +void test() { + foo<float>(9); // expected-error{{use of floating-point or vector values is disabled}} expected-note{{in instantiation of function template specialization}} + foo<float>(); // expected-error{{use of floating-point or vector values is disabled}} expected-note{{in instantiation of default function argument}} expected-note{{from use of default argument}} +} +} // namespace templates + +namespace base_classes { +struct Foo { + BannedTy f; +}; + +struct Bar : Foo {}; +struct Baz : virtual Foo {}; + +struct Nothing {}; +struct Qux : Nothing, Baz {}; + +Foo getFoo() { // expected-error{{use of floating-point or vector values is disabled}} + __builtin_trap(); +} +Bar getBar() { // expected-error{{use of floating-point or vector values is disabled}} + __builtin_trap(); +} +Baz getBaz() { // expected-error{{use of floating-point or vector values is disabled}} + __builtin_trap(); +} +Qux getQux() { // expected-error{{use of floating-point or vector values is disabled}} + __builtin_trap(); +} +} // namespace base_classes + +namespace constexprs { +constexpr float foo = 1.0; +constexpr int bar = 1.0; + +constexpr int getFoo() { return 1; } + +constexpr float baz = getFoo(); +constexpr int qux = baz + foo + bar; + +int getVal() { + return qux; +} + +int getVal2() { + return baz; +} + +constexpr int transformFoo(float foo) { // expected-error{{use of floating-point or vector values is disabled}} + return foo + 1; // expected-error{{use of floating-point or vector values is disabled}} +} + +int getVal3() { + constexpr int val = transformFoo(1); + int val2 = transformFoo(1); // expected-error{{use of floating-point or vector values is disabled}} +} +} // namespace constexprs Index: clang/test/Sema/aarch64-mgeneral_regs_only.c =================================================================== --- /dev/null +++ clang/test/Sema/aarch64-mgeneral_regs_only.c @@ -0,0 +1,251 @@ +// RUN: %clang_cc1 -triple aarch64-linux-eabi -general-regs-only %s -verify -DBANNED=int '-DVECATTR=__attribute__((ext_vector_type(2)))' -Wno-unused-value +// RUN: %clang_cc1 -triple aarch64-linux-eabi -general-regs-only %s -verify -DBANNED=float -Wno-unused-value +// RUN: %clang_cc1 -triple aarch64-linux-eabi -general-regs-only %s -verify -DBANNED=FloatTypedef -Wno-unused-value + +// Try to diagnose every use of a floating-point or vector operation that we +// can't trivially fold. Declaring these, passing their addresses around, etc. +// is OK, assuming we never actually use them in this TU. + +typedef float FloatTypedef; + +#ifndef VECATTR +#define VECATTR +#endif +typedef BANNED BannedTy VECATTR; + +// Whether or not this is the actual definition for uintptr_t doesn't matter. +typedef unsigned long long uintptr_t; + +// We only start caring when the user actually tries to do things with floats; +// declarations on their own are fine. +// This allows a user to #include some headers that happen to use floats. As +// long as they're never actually used, no one cares. +BannedTy foo(); +void bar(BannedTy); +extern BannedTy gBaz; + +void calls() { + __auto_type a = foo(); // expected-error{{use of floating-point or vector values is disabled}} + BannedTy b = foo(); // expected-error{{use of floating-point or vector values is disabled}} + foo(); // expected-error{{use of floating-point or vector values is disabled}} + (void)foo(); // expected-error{{use of floating-point or vector values is disabled}} + + bar(1); // expected-error{{use of floating-point or vector values is disabled}} + bar(1.); // expected-error{{use of floating-point or vector values is disabled}} + + gBaz = 1; // expected-error{{use of floating-point or vector values is disabled}} + gBaz = 1.; // expected-error{{use of floating-point or vector values is disabled}} +} + +BannedTy global_banned; + +void literals() { + volatile int i; + i = 1.; + i = 1.0 + 2; + i = (int)(1.0 + 2); + + BannedTy j = 1; // expected-error{{use of floating-point or vector values is disabled}} + BannedTy k = (BannedTy)1.1; // expected-error{{use of floating-point or vector values is disabled}} + BannedTy l; + (BannedTy)3; // expected-error{{use of floating-point or vector values is disabled}} +} + +struct Baz { + int i; + BannedTy f; +}; + +union Qux { + int i; + BannedTy j; + BannedTy *p; +}; + +struct Baz *getBaz(int i); + +void structs(void *p) { + struct Baz b; + union Qux q; + + q = (union Qux){}; + q.i = 1; + q.j = 2.; // expected-error{{use of floating-point or vector values is disabled}} + q.j = 2.f; // expected-error{{use of floating-point or vector values is disabled}} + q.j = 2; // expected-error{{use of floating-point or vector values is disabled}} + q.p = (BannedTy *)p; + q.p += 5; + *q.p += 5; // expected-error{{use of floating-point or vector values is disabled}} + + b = (struct Baz){}; // expected-error{{use of floating-point or vector values is disabled}} + b = *getBaz(2. + b.i); // expected-error{{use of floating-point or vector values is disabled}} + *getBaz(2. + b.i) = b; // expected-error{{use of floating-point or vector values is disabled}} +} + +struct Baz callBaz(struct Baz); +union Qux callQux(union Qux); +struct Baz *callBazPtr(struct Baz *); +union Qux *callQuxPtr(union Qux *); + +void structCalls() { + void *p; + callBazPtr((struct Baz *)p); + callQuxPtr((union Qux *)p); + + // One error for returning a `struct Baz`, one for taking a `struct Baz`, one + // for constructing a `struct Baz`. Not ideal, but... + callBaz((struct Baz){}); // expected-error 3{{use of floating-point or vector values is disabled}} + callQux((union Qux){}); +} + +extern BannedTy extern_arr[4]; +static BannedTy static_arr[4]; + +void arrays() { + BannedTy bannedArr[] = { // expected-error{{use of floating-point or vector values is disabled}} + 1, + 1.0, + 2.0f, + }; + int intArr[] = {1.0, 2.0f}; + + intArr[0] = 1.0; + bannedArr[0] = 1; // expected-error{{use of floating-point or vector values is disabled}} +} + +BannedTy *getMemberPtr(struct Baz *b, int i) { + if (i) + return &b->f; + return &((struct Baz *)(uintptr_t)((uintptr_t)b + 1.0))->f; // expected-error{{use of floating-point or vector values is disabled}} +} + +void casts() { + void *volatile p; + + (BannedTy *)p; + (void)*(BannedTy *)p; // expected-error{{use of floating-point or vector values is disabled}} + + (void)*(struct Baz *)p; // expected-error{{use of floating-point or vector values is disabled}} + (void)*(union Qux *)p; + (void)((union Qux *)p)->i; + (void)((union Qux *)p)->j; // expected-error{{use of floating-point or vector values is disabled}} +} + +BannedTy returns() { // expected-error{{use of floating-point or vector values is disabled}} + return 0; // expected-error{{use of floating-point or vector values is disabled}} +} + +int unevaluated() { + return sizeof((BannedTy)0.0); +} + +void moreUnevaluated(int x) + __attribute__((diagnose_if(x + 1.1 == 2.1, "oh no", "warning"))) { + moreUnevaluated(3); + moreUnevaluated(1); // expected-warning{{oh no}} expected-note@-2{{from 'diagnose_if'}} +} + +void noSpam() { + float r = 1. + 2 + 3 + 4 + 5.; // expected-error 2{{use of floating-point or vector values is disabled}} + float r2 = 1. + r + 3 + 4 + 5.; // expected-error 3{{use of floating-point or vector values is disabled}} + float r3 = 1 + 2 + 3 + 4 + 5; // expected-error{{use of floating-point or vector values is disabled}} + + // no errors expected below: they can be trivially folded to a constant. + int i = 1. + 2 + 3 + 4 + 5.; // no error: we can trivially fold this to a constant. + int j = (int)(1. + 2 + 3) + 4; // no error: we can trivially fold this to a constant. + int k = (int)(1. + 2 + 3) + j; + int l = (int)(1. + 2 + 3) + + r; //expected-error {{use of floating-point or vector values is disabled}} + + const int cj = (int)(1. + 2 + 3) + 4; // no error: we can trivially fold this to a constant. + int ck = (int)(1. + cj + 3) + + r; // expected-error{{use of floating-point or vector values is disabled}} +} + +float fooFloat(); +int exprStmt() { + return ({ fooFloat() + 1 + 2; }) + 3; // expected-error 2{{use of floating-point or vector values is disabled}} +} + +int longExprs() { + return 1 + ((struct Baz){.i = 1}).i; // expected-error{{use of floating-point or vector values is disabled}} +} + +struct RecursiveTy { // expected-note{{is not complete until}} + int a; + BannedTy b; + struct RecursiveTy ty; // expected-error{{field has incomplete type}} +}; + +struct StructRec { + int a; + BannedTy b; + struct RecursiveTy ty; +}; + +union UnionRec { + int a; + BannedTy b; + struct RecursiveTy ty; +}; + +struct UndefType; // expected-note 3{{forward declaration}} + +struct StructUndef { + int a; + BannedTy b; + struct UndefType s; // expected-error{{field has incomplete type}} +}; + +union UnionUndef { + int a; + BannedTy b; + struct UndefType s; // expected-error{{field has incomplete type}} +}; + +// ...Just be sure we don't crash on these. +void cornerCases() { + struct RecInl { // expected-note{{is not complete until the closing}} + struct RecInl s; // expected-error{{field has incomplete type}} + BannedTy f; + } inl; + __builtin_memset(&inl, 0, sizeof(inl)); + + BannedTy fs[] = { + ((struct RecursiveTy){}).a, + ((struct StructRec){}).a, + ((union UnionRec){}).a, + ((struct StructUndef){}).a, + ((union UnionUndef){}).a, + }; + + BannedTy fs2[] = { + ((struct RecursiveTy){}).b, + ((struct StructRec){}).b, + ((union UnionRec){}).b, + ((struct StructUndef){}).b, + ((union UnionUndef){}).b, + }; + + struct UndefType a = {}; // expected-error{{has incomplete type}} + struct RecursiveTy b = {}; + struct StructRec c = {}; + union UnionRec d = {}; + struct StructUndef e = {}; + union UnionUndef f = {}; + + __builtin_memset(&a, 0, sizeof(a)); + __builtin_memset(&b, 0, sizeof(b)); + __builtin_memset(&c, 0, sizeof(c)); + __builtin_memset(&d, 0, sizeof(d)); + __builtin_memset(&e, 0, sizeof(e)); + __builtin_memset(&f, 0, sizeof(f)); +} + +void floatFunctionDefIn(BannedTy a) {} // expected-error{{use of floating-point or vector values is disabled}} +BannedTy floatFunctionDefOut(void) {} // expected-error{{use of floating-point or vector values is disabled}} +BannedTy // expected-error{{use of floating-point or vector values is disabled}} +floatFunctionDefInOut(BannedTy a) {} // expected-error{{use of floating-point or vector values is disabled}} + +void floatInStructDefIn(struct Baz a) {} // expected-error{{use of floating-point or vector values is disabled}} +struct Baz floatInStructDefOut(void) {} // expected-error{{use of floating-point or vector values is disabled}} Index: clang/test/CodeGen/aarch64-mgeneral_regs_only.c =================================================================== --- /dev/null +++ clang/test/CodeGen/aarch64-mgeneral_regs_only.c @@ -0,0 +1,68 @@ +// RUN: %clang -target aarch64 -mgeneral-regs-only %s -o - -S -emit-llvm | FileCheck %s +// %clang -target aarch64 -mgeneral-regs-only %s -o - -S | FileCheck %s --check-prefix=ASM + +// CHECK-LABEL: @j = dso_local constant i32 3 + +float arr[8]; + +// CHECK-LABEL: noimplicitfloat +// CHECK-NEXT: define dso_local void @foo +const int j = 1.0 + 2.0; +void foo(int *i) { + // CHECK: store i32 4 + *i = 1.0 + j; +} + +// CHECK-LABEL: noimplicitfloat +// CHECK-NEXT: define dso_local void @bar +void bar(float **j) { + __builtin_memcpy(arr, j, sizeof(arr)); + *j = arr; +} + +struct OneFloat { + float i; +}; +struct TwoFloats { + float i, j; +}; + +static struct OneFloat oneFloat; +static struct TwoFloats twoFloats; + +// CHECK-LABEL: testOneFloat +void testOneFloat(const struct OneFloat *o, struct OneFloat *f) { + // These memcpys are necessary, so we don't generate FP ops in LLVM. + // CHECK: @llvm.memcpy + // CHECK: @llvm.memcpy + *f = *o; + oneFloat = *o; +} + +// CHECK-LABEL: testTwoFloats +void testTwoFloats(const struct TwoFloats *o, struct TwoFloats *f) { + // CHECK: @llvm.memcpy + // CHECK: @llvm.memcpy + *f = *o; + twoFloats = *o; +} + +// -mgeneral-regs-only implies that the compiler can't invent +// floating-point/vector instructions. The user should be allowed to do that, +// though. +// +// CHECK-LABEL: baz +// ASM-LABEL: baz: +void baz(float *i) { + // CHECK: call float* asm sideeffect + // ASM: fmov s1, # + // ASM: fadd s0, s0, s1 + asm volatile("ldr s0, [%0]\n" + "fmov s1, #1.00000000\n" + "fadd s0, s0, s1\n" + "str s0, [%0]\n" + "ret\n" + : "=r"(i) + : + : "s0", "s1"); +} Index: clang/lib/Sema/SemaExprCXX.cpp =================================================================== --- clang/lib/Sema/SemaExprCXX.cpp +++ clang/lib/Sema/SemaExprCXX.cpp @@ -19,6 +19,7 @@ #include "clang/AST/CXXInheritance.h" #include "clang/AST/CharUnits.h" #include "clang/AST/DeclObjC.h" +#include "clang/AST/EvaluatedExprVisitor.h" #include "clang/AST/ExprCXX.h" #include "clang/AST/ExprObjC.h" #include "clang/AST/RecursiveASTVisitor.h" @@ -7898,6 +7899,253 @@ return E; } +static bool typeHasFloatingOrVectorComponent(QualType Ty); + +static bool recordHasFloatingOrVectorComponent(const RecordDecl *Record) { + Record = Record->getDefinition(); + // Be conservative in the face of broken code (e.g. undefined types, + // recursive types, ...) + if (!Record || Record->isInvalidDecl()) + return false; + + auto FieldHasFPOrVectorComponent = [](const FieldDecl *FD) { + return typeHasFloatingOrVectorComponent(FD->getType()); + }; + + // We treat any union with mixed FP/vector and non-FP/vector elements as a bag + // of bits. + if (Record->isUnion()) + return llvm::all_of(Record->fields(), FieldHasFPOrVectorComponent); + + if (llvm::any_of(Record->fields(), FieldHasFPOrVectorComponent)) + return true; + + if (const auto *CXXRD = dyn_cast<CXXRecordDecl>(Record)) + return llvm::any_of(CXXRD->bases(), [](const CXXBaseSpecifier &Base) { + return typeHasFloatingOrVectorComponent(Base.getType()); + }); + + return false; +} + +static bool typeHasFloatingOrVectorComponent(QualType Ty) { + if (Ty.isNull()) + return false; + + while (const auto *AT = Ty->getAsArrayTypeUnsafe()) + Ty = AT->getElementType(); + + if (Ty->isScalarType()) + return Ty->hasFloatingRepresentation(); + + if (Ty->isVectorType()) + return true; + + if (const auto *Record = Ty.getCanonicalType()->getAsRecordDecl()) + return recordHasFloatingOrVectorComponent(Record); + + return false; +} + +bool Sema::typeHasFloatingOrVectorComponent(QualType Ty) { + return ::typeHasFloatingOrVectorComponent(Ty.getCanonicalType()); +} + +namespace { +// Diagnoses any uses of vector/floating-point values that we aren't guaranteed +// to fold away in codegen. +class NonGeneralOpDiagnoser + : public ConstEvaluatedExprVisitor<NonGeneralOpDiagnoser> { + using Super = ConstEvaluatedExprVisitor<NonGeneralOpDiagnoser>; + + struct DiagnosticInfo { + SourceLocation Loc; + SourceRange Range; + + // Default arguments are only diagnosed when they're used, but the error + // diagnostic points to the default argument itself. This contains the + // series of calls that brought us to that default arg. + llvm::TinyPtrVector<const CallExpr *> DefaultArgLocs; + + // These should only exist in their fully-constructed form. + DiagnosticInfo() = delete; + + DiagnosticInfo(const Expr *E) + : Loc(E->getBeginLoc()), Range(E->getSourceRange()) {} + + // There's nothing inherently _wrong_ about copying these, but there's also + // no reason to want to do so now. Since it involves an implicit clone of a + // vector, discourage it. + DiagnosticInfo(const DiagnosticInfo &) = delete; + DiagnosticInfo &operator=(const DiagnosticInfo &) = delete; + + DiagnosticInfo(DiagnosticInfo &&) = default; + DiagnosticInfo &operator=(DiagnosticInfo &&) = default; + }; + + llvm::SmallVector<DiagnosticInfo, 8> DiagnosticsToEmit; + + bool isGeneralType(const Expr *E) const { + return !typeHasFloatingOrVectorComponent(E->getType().getCanonicalType()); + } + + void enqueueDiagnosticFor(const Expr *E) { + DiagnosticsToEmit.emplace_back(E); + } + + bool isRValueOfIllegalType(const Expr *E) const { + return E->getValueKind() != VK_LValue && !isGeneralType(E); + } + + bool diagnoseIfNonGeneralRValue(const Expr *E) { + if (!isRValueOfIllegalType(E)) + return false; + + enqueueDiagnosticFor(E); + return true; + } + + static const Expr *ignoreParenImpFloatCastsAndSplats(const Expr *E) { + const Expr *Cur = E->IgnoreParensExceptConstantExpr(); + if (const auto *Sub = dyn_cast<ImplicitCastExpr>(Cur)) + if (Sub->getCastKind() == CK_IntegralToFloating || + Sub->getCastKind() == CK_VectorSplat) + return Sub->getSubExpr()->IgnoreParensExceptConstantExpr(); + return Cur; + } + + struct DiagnosticState { + unsigned InitialSize; + }; + + DiagnosticState saveDiagnosticState() const { + return {static_cast<unsigned>(DiagnosticsToEmit.size())}; + } + + MutableArrayRef<DiagnosticInfo> + diagnosticsIssuedSince(const DiagnosticState &S) { + assert(S.InitialSize <= DiagnosticsToEmit.size() && + "DiagnosticsToEmit shouldn't shrink across saves!"); + return {DiagnosticsToEmit.begin() + S.InitialSize, DiagnosticsToEmit.end()}; + } + + void resetDiagnosticState(const DiagnosticState &S) { + DiagnosticsToEmit.erase(DiagnosticsToEmit.begin() + S.InitialSize, + DiagnosticsToEmit.end()); + } + + // Special case: Don't diagnose patterns that will ultimately turn into a + // memcpy in CodeGen. Returns true if we matched the pattern (which may still + // result in diagnostics on subexpressions being emitted). This is a small + // convenience to the user, so they don't have to write out memcpy() for + // simple cases. For that reason, it's very restricted in what it detects. + // + // N.B. this is C-specific, since C++ doesn't really deal in assignments of + // structs. + bool diagnoseTrivialRecordAssignmentExpr(const BinaryOperator *BO) { + if (BO->getOpcode() != BO_Assign || !BO->getType()->isRecordType()) + return false; + + const auto *RHS = dyn_cast<ImplicitCastExpr>(BO->getRHS()->IgnoreParens()); + if (!RHS || RHS->getCastKind() != CK_LValueToRValue) + return false; + + Visit(BO->getLHS()); + Visit(RHS->getSubExpr()); + return true; + } + +public: + NonGeneralOpDiagnoser(Sema &S) : Super(S.getASTContext()){} + + void VisitExpr(const Expr *E) { + if (!diagnoseIfNonGeneralRValue(E)) + Super::VisitExpr(E); + } + + void VisitConstantExpr(const ConstantExpr *E) { + // If this is guaranteed to be constant-folded away to nothing but the + // value of E, we don't care what went into building it, as long as the + // result is an allowed type. + diagnoseIfNonGeneralRValue(E); + } + + void VisitCXXDefaultArgExpr(const CXXDefaultArgExpr *E) { + // Note that simply calling getExpr() is incorrect, since getExpr() looks + // through ConstantExpr nodes. + Visit(E->getParam()->getInit()); + } + + void VisitCallExpr(const CallExpr *E) { + diagnoseIfNonGeneralRValue(E); + Visit(E->getCallee()); + + for (const Expr *Arg : E->arguments()) { + DiagnosticState Saved = saveDiagnosticState(); + Visit(Arg); + if (Arg->isDefaultArgument()) + for (DiagnosticInfo &DI : diagnosticsIssuedSince(Saved)) + DI.DefaultArgLocs.push_back(E); + } + } + + void VisitCastExpr(const CastExpr *E) { + Super::VisitExpr(E); + if (isRValueOfIllegalType(E) && !isRValueOfIllegalType(E->getSubExpr())) + enqueueDiagnosticFor(E); + } + + void VisitParenExpr(const ParenExpr *E) { Visit(E->getSubExpr()); } + void VisitDeclRefExpr(const DeclRefExpr *E) { diagnoseIfNonGeneralRValue(E); } + + void VisitBinaryOperator(const BinaryOperator *BO) { + if (isGeneralType(BO)) { + Visit(BO->getLHS()); + Visit(BO->getRHS()); + return; + } + + if (diagnoseTrivialRecordAssignmentExpr(BO)) + return; + + DiagnosticState InitialState = saveDiagnosticState(); + // Ignore implicit casts to disallowed types so we minimize diagnostics for + // things like `1 + 2. + 3 + 4`. + Visit(ignoreParenImpFloatCastsAndSplats(BO->getLHS())); + Visit(ignoreParenImpFloatCastsAndSplats(BO->getRHS())); + + // Since we don't diagnose LValues, this is somewhat common. + if (diagnosticsIssuedSince(InitialState).empty()) + enqueueDiagnosticFor(BO); + } + + void VisitExprWithCleanups(const ExprWithCleanups *E) { + Visit(E->getSubExpr()); + } + + static void diagnoseExpr(Sema &S, const Expr *E) { + NonGeneralOpDiagnoser Diagnoser(S); + Diagnoser.Visit(E); + for (const DiagnosticInfo &DI : Diagnoser.DiagnosticsToEmit) { + SourceLocation Loc = DI.Loc; + SourceRange Range = DI.Range; + // There are rare cases (e.g. `(struct Foo){}`, where Foo + // isNonGeneralType), where the expression we're trying to point to + // doesn't exist. Fall back to complaining about the full expr. + if (Loc.isInvalid()) { + Loc = E->getBeginLoc(); + Range = E->getSourceRange(); + assert(!Loc.isInvalid()); + } + S.Diag(Loc, diag::err_non_general_ops_disabled) << Range; + + for (const CallExpr *CE : DI.DefaultArgLocs) + S.Diag(CE->getRParenLoc(), diag::note_from_use_of_default_arg); + } + } +}; +} // end anonymous namespace + ExprResult Sema::ActOnFinishFullExpr(Expr *FE, SourceLocation CC, bool DiscardedValue, bool IsConstexpr) { @@ -7935,6 +8183,9 @@ CheckCompletedExpr(FullExpr.get(), CC, IsConstexpr); + if (!IsConstexpr && getLangOpts().GeneralRegsOnly) + NonGeneralOpDiagnoser::diagnoseExpr(*this, FullExpr.get()); + // At the end of this full expression (which could be a deeply nested // lambda), if there is a potential capture within the nested lambda, // have the outer capture-able lambda try and capture it. Index: clang/lib/Sema/SemaDecl.cpp =================================================================== --- clang/lib/Sema/SemaDecl.cpp +++ clang/lib/Sema/SemaDecl.cpp @@ -8978,6 +8978,19 @@ // Finally, we know we have the right number of parameters, install them. NewFD->setParams(Params); + if (getLangOpts().GeneralRegsOnly && + D.getFunctionDefinitionKind() == FDK_Definition) { + if (typeHasFloatingOrVectorComponent(NewFD->getReturnType())) { + SourceRange Range = NewFD->getReturnTypeSourceRange(); + Diag(Range.getBegin(), diag::err_non_general_ops_disabled) << Range; + } + + for (const ParmVarDecl *PVD : NewFD->parameters()) + if (typeHasFloatingOrVectorComponent(PVD->getType())) + Diag(PVD->getBeginLoc(), diag::err_non_general_ops_disabled) + << PVD->getSourceRange(); + } + if (D.getDeclSpec().isNoreturnSpecified()) NewFD->addAttr( ::new(Context) C11NoReturnAttr(D.getDeclSpec().getNoreturnSpecLoc(), Index: clang/lib/Sema/SemaCast.cpp =================================================================== --- clang/lib/Sema/SemaCast.cpp +++ clang/lib/Sema/SemaCast.cpp @@ -109,7 +109,7 @@ castExpr->getValueKind()); } updatePartOfExplicitCastFlags(castExpr); - return castExpr; + return Self.wrapInConstantExprIfNecessary(castExpr); } // Internal convenience methods. @@ -2881,6 +2881,23 @@ << TheOffendingSrcType << TheOffendingDestType << qualifiers; } +Expr *Sema::wrapInConstantExprIfNecessary(CastExpr *CE) const { + if (!getLangOpts().GeneralRegsOnly || CE->isInstantiationDependent()) + return CE; + + // As far as we're concerned here, ConstantExpr is only useful to force us to + // never emit code manipulating values with illegal types. + if (typeHasFloatingOrVectorComponent(CE->getSubExpr()->getType()) && + !typeHasFloatingOrVectorComponent(CE->getType())) { + Expr::EvalResult Result; + if (CE->EvaluateAsConstantExpr(Result, Expr::EvaluateForCodeGen, + getASTContext())) + return ConstantExpr::Create(getASTContext(), CE, Result.Val); + } + + return CE; +} + ExprResult Sema::BuildCStyleCastExpr(SourceLocation LPLoc, TypeSourceInfo *CastTypeInfo, SourceLocation RPLoc, Index: clang/lib/Sema/Sema.cpp =================================================================== --- clang/lib/Sema/Sema.cpp +++ clang/lib/Sema/Sema.cpp @@ -549,11 +549,12 @@ if (ImpCast->getCastKind() == Kind && (!BasePath || BasePath->empty())) { ImpCast->setType(Ty); ImpCast->setValueKind(VK); - return E; + return wrapInConstantExprIfNecessary(ImpCast); } } - return ImplicitCastExpr::Create(Context, Ty, Kind, E, BasePath, VK); + return wrapInConstantExprIfNecessary( + ImplicitCastExpr::Create(Context, Ty, Kind, E, BasePath, VK)); } /// ScalarTypeToBooleanCastKind - Returns the cast kind corresponding Index: clang/lib/Frontend/CompilerInvocation.cpp =================================================================== --- clang/lib/Frontend/CompilerInvocation.cpp +++ clang/lib/Frontend/CompilerInvocation.cpp @@ -2605,6 +2605,9 @@ if (Args.hasArg(OPT_pthread)) Opts.POSIXThreads = 1; + if (Args.hasArg(OPT_general_regs_only)) + Opts.GeneralRegsOnly = 1; + // The value-visibility mode defaults to "default". if (Arg *visOpt = Args.getLastArg(OPT_fvisibility)) { Opts.setValueVisibilityMode(parseVisibility(visOpt, Args, Diags)); Index: clang/lib/Driver/ToolChains/Clang.cpp =================================================================== --- clang/lib/Driver/ToolChains/Clang.cpp +++ clang/lib/Driver/ToolChains/Clang.cpp @@ -1396,6 +1396,24 @@ } } +static void addGeneralRegsOnlyArgs(const Driver &D, const ArgList &Args, + ArgStringList &CmdArgs) { + if (Args.getLastArg(options::OPT_mgeneral_regs_only)) { + if (Args.hasFlag(options::OPT_mimplicit_float, + options::OPT_mno_implicit_float, false)) { + D.Diag(diag::err_drv_argument_not_allowed_with) << "-mimplicit-float" + << "-mgeneral-regs-only"; + return; + } + + CmdArgs.push_back("-general-regs-only"); + CmdArgs.push_back("-no-implicit-float"); + } else if (!Args.hasFlag(options::OPT_mimplicit_float, + options::OPT_mno_implicit_float, true)) { + CmdArgs.push_back("-no-implicit-float"); + } +} + void Clang::AddARMTargetArgs(const llvm::Triple &Triple, const ArgList &Args, ArgStringList &CmdArgs, bool KernelOrKext) const { RenderARMABI(Triple, Args, CmdArgs); @@ -1429,9 +1447,7 @@ CmdArgs.push_back("-arm-global-merge=true"); } - if (!Args.hasFlag(options::OPT_mimplicit_float, - options::OPT_mno_implicit_float, true)) - CmdArgs.push_back("-no-implicit-float"); + addGeneralRegsOnlyArgs(getToolChain().getDriver(), Args, CmdArgs); if (Args.getLastArg(options::OPT_mcmse)) CmdArgs.push_back("-mcmse"); @@ -1588,9 +1604,7 @@ Args.hasArg(options::OPT_fapple_kext)) CmdArgs.push_back("-disable-red-zone"); - if (!Args.hasFlag(options::OPT_mimplicit_float, - options::OPT_mno_implicit_float, true)) - CmdArgs.push_back("-no-implicit-float"); + addGeneralRegsOnlyArgs(getToolChain().getDriver(), Args, CmdArgs); RenderAArch64ABI(Triple, Args, CmdArgs); Index: clang/lib/Driver/ToolChains/Arch/AArch64.cpp =================================================================== --- clang/lib/Driver/ToolChains/Arch/AArch64.cpp +++ clang/lib/Driver/ToolChains/Arch/AArch64.cpp @@ -192,6 +192,9 @@ Features.push_back("-fp-armv8"); Features.push_back("-crypto"); Features.push_back("-neon"); + // FIXME: Ideally, we'd also like to pass a `+general-regs-only` feature, + // and do extra checking in the backend to be sure that we haven't + // accidentally introduced any FP ops. } if (Arg *A = Args.getLastArg(options::OPT_mtp_mode_EQ)) { Index: clang/lib/AST/Expr.cpp =================================================================== --- clang/lib/AST/Expr.cpp +++ clang/lib/AST/Expr.cpp @@ -2874,6 +2874,7 @@ return E; } +template <bool IgnoreConstantExpr = true> static Expr *IgnoreParensSingleStep(Expr *E) { if (auto *PE = dyn_cast<ParenExpr>(E)) return PE->getSubExpr(); @@ -2893,8 +2894,10 @@ return CE->getChosenSubExpr(); } - else if (auto *CE = dyn_cast<ConstantExpr>(E)) - return CE->getSubExpr(); + else if (IgnoreConstantExpr) { + if (auto *CE = dyn_cast<ConstantExpr>(E)) + return CE->getSubExpr(); + } return E; } @@ -2953,17 +2956,21 @@ return IgnoreExprNodes(this, IgnoreImplicitSingleStep); } +Expr *Expr::IgnoreParensExceptConstantExpr() { + return IgnoreExprNodes(this, IgnoreParensSingleStep<false>); +} + Expr *Expr::IgnoreParens() { - return IgnoreExprNodes(this, IgnoreParensSingleStep); + return IgnoreExprNodes(this, IgnoreParensSingleStep<>); } Expr *Expr::IgnoreParenImpCasts() { - return IgnoreExprNodes(this, IgnoreParensSingleStep, + return IgnoreExprNodes(this, IgnoreParensSingleStep<>, IgnoreImpCastsExtraSingleStep); } Expr *Expr::IgnoreParenCasts() { - return IgnoreExprNodes(this, IgnoreParensSingleStep, IgnoreCastsSingleStep); + return IgnoreExprNodes(this, IgnoreParensSingleStep<>, IgnoreCastsSingleStep); } Expr *Expr::IgnoreConversionOperator() { @@ -2975,17 +2982,17 @@ } Expr *Expr::IgnoreParenLValueCasts() { - return IgnoreExprNodes(this, IgnoreParensSingleStep, + return IgnoreExprNodes(this, IgnoreParensSingleStep<>, IgnoreLValueCastsSingleStep); } Expr *Expr::ignoreParenBaseCasts() { - return IgnoreExprNodes(this, IgnoreParensSingleStep, + return IgnoreExprNodes(this, IgnoreParensSingleStep<>, IgnoreBaseCastsSingleStep); } Expr *Expr::IgnoreParenNoopCasts(const ASTContext &Ctx) { - return IgnoreExprNodes(this, IgnoreParensSingleStep, [&Ctx](Expr *E) { + return IgnoreExprNodes(this, IgnoreParensSingleStep<>, [&Ctx](Expr *E) { return IgnoreNoopCastsSingleStep(Ctx, E); }); } Index: clang/include/clang/Sema/Sema.h =================================================================== --- clang/include/clang/Sema/Sema.h +++ clang/include/clang/Sema/Sema.h @@ -10949,7 +10949,19 @@ SourceLocation getLocationOfStringLiteralByte(const StringLiteral *SL, unsigned ByteNo) const; + // Either wrap the given cast in a ConstantExpr node, or return the cast + // unmodified. + // + // This is used in compilation modes where we can't emit values of certain + // types (e.g., -mgeneral-regs-only), but where we want to be able to fold + // these unlowerable values into constants of lowerable types. + Expr *wrapInConstantExprIfNecessary(CastExpr *CE) const; + private: + // Tests to see if the given type is or contains a float or vector, as defined + // by -mgeneral-regs-only. + static bool typeHasFloatingOrVectorComponent(QualType Ty); + void CheckArrayAccess(const Expr *BaseExpr, const Expr *IndexExpr, const ArraySubscriptExpr *ASE=nullptr, bool AllowOnePastEnd=true, bool IndexNegated=false); Index: clang/include/clang/Driver/CC1Options.td =================================================================== --- clang/include/clang/Driver/CC1Options.td +++ clang/include/clang/Driver/CC1Options.td @@ -246,6 +246,8 @@ HelpText<"Emit an error if a C++ static local initializer would need a guard variable">; def no_implicit_float : Flag<["-"], "no-implicit-float">, HelpText<"Don't generate implicit floating point instructions">; +def general_regs_only : Flag<["-"], "general-regs-only">, + HelpText<"Don't allow the generation of floating point or vector instructions">; def fdump_vtable_layouts : Flag<["-"], "fdump-vtable-layouts">, HelpText<"Dump the layouts of all vtables that will be emitted in a translation unit">; def fmerge_functions : Flag<["-"], "fmerge-functions">, Index: clang/include/clang/Basic/LangOptions.def =================================================================== --- clang/include/clang/Basic/LangOptions.def +++ clang/include/clang/Basic/LangOptions.def @@ -141,6 +141,7 @@ LANGOPT(Coroutines , 1, 0, "C++20 coroutines") LANGOPT(DllExportInlines , 1, 1, "dllexported classes dllexport inline methods") LANGOPT(RelaxedTemplateTemplateArgs, 1, 0, "C++17 relaxed matching of template template arguments") +LANGOPT(GeneralRegsOnly , 1, 0, "Whether to diagnose the use of floating-point or vector operations") LANGOPT(DoubleSquareBracketAttributes, 1, 0, "'[[]]' attributes extension for all language standard modes") Index: clang/include/clang/Basic/DiagnosticSemaKinds.td =================================================================== --- clang/include/clang/Basic/DiagnosticSemaKinds.td +++ clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -6277,6 +6277,11 @@ def ext_freestanding_complex : Extension< "complex numbers are an extension in a freestanding C99 implementation">; +def err_non_general_ops_disabled : Error< + "use of floating-point or vector values is disabled by " + "'-mgeneral-regs-only'">; +def note_from_use_of_default_arg : Note<"from use of default argument here">; + // FIXME: Remove when we support imaginary. def err_imaginary_not_supported : Error<"imaginary types are not supported">; Index: clang/include/clang/AST/Expr.h =================================================================== --- clang/include/clang/AST/Expr.h +++ clang/include/clang/AST/Expr.h @@ -792,7 +792,13 @@ /// * UnaryOperator if `UO_Extension` /// * GenericSelectionExpr if `!isResultDependent()` /// * ChooseExpr if `!isConditionDependent()` - /// * ConstantExpr + Expr *IgnoreParensExceptConstantExpr() LLVM_READONLY; + const Expr *IgnoreParensExceptConstantExpr() const { + return const_cast<Expr *>(this)->IgnoreParensExceptConstantExpr(); + } + + /// Identical to IgnoreParensExceptConstantExpr, though also skips + /// ConstantExprs. Expr *IgnoreParens() LLVM_READONLY; const Expr *IgnoreParens() const { return const_cast<Expr *>(this)->IgnoreParens(); Index: clang/docs/UsersManual.rst =================================================================== --- clang/docs/UsersManual.rst +++ clang/docs/UsersManual.rst @@ -1372,7 +1372,8 @@ Generate code which only uses the general purpose registers. This option restricts the generated code to use general registers - only. This only applies to the AArch64 architecture. + only. This only applies to the AArch64 architecture. This restriction does + not apply to inline assembly. .. option:: -mcompact-branches=[values]
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits