diff --git a/clang/include/clang/AST/DeclCXX.h b/clang/include/clang/AST/DeclCXX.h index 49a0e6e98262e..a5dc83735fde0 100644 --- a/clang/include/clang/AST/DeclCXX.h +++ b/clang/include/clang/AST/DeclCXX.h @@ -2555,9 +2555,9 @@ class CXXConstructorDecl final ExplicitSpecifier getExplicitSpecifierInternal() const { if (CXXConstructorDeclBits.HasTrailingExplicitSpecifier) - return *getCanonicalDecl()->getTrailingObjects(); + return *getTrailingObjects(); return ExplicitSpecifier( - nullptr, getCanonicalDecl()->CXXConstructorDeclBits.IsSimpleExplicit + nullptr, CXXConstructorDeclBits.IsSimpleExplicit ? ExplicitSpecKind::ResolvedTrue : ExplicitSpecKind::ResolvedFalse); } @@ -2598,10 +2598,10 @@ class CXXConstructorDecl final InheritedConstructor Inherited = InheritedConstructor()); ExplicitSpecifier getExplicitSpecifier() { - return getExplicitSpecifierInternal(); + return getCanonicalDecl()->getExplicitSpecifierInternal(); } const ExplicitSpecifier getExplicitSpecifier() const { - return getExplicitSpecifierInternal(); + return getCanonicalDecl()->getExplicitSpecifierInternal(); } /// Return true if the declartion is already resolved to be explicit. diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index ccf2a60dad1f7..965cccc480a30 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -6137,6 +6137,10 @@ def warn_out_of_range_compare : Warning< InGroup; def warn_tautological_bool_compare : Warning, InGroup; +def warn_integer_constants_in_conditional_always_true : Warning< + "converting the result of '?:' with integer constants to a boolean always " + "evaluates to 'true'">, + InGroup; def warn_comparison_of_mixed_enum_types : Warning< "comparison of two values with different enumeration types" "%diff{ ($ and $)|}0,1">, diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index ddf58aba1eb9f..7aeace5c2a7d5 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -11256,6 +11256,44 @@ static bool isSameWidthConstantConversion(Sema &S, Expr *E, QualType T, return true; } +static void DiagnoseIntInBoolContext(Sema &S, const Expr *E) { + E = E->IgnoreParenImpCasts(); + SourceLocation ExprLoc = E->getExprLoc(); + + if (const auto *CO = dyn_cast(E)) { + const auto *LHS = dyn_cast(CO->getTrueExpr()); + if (!LHS) { + if (auto *UO = dyn_cast(CO->getTrueExpr())) { + if (UO->getOpcode() == UO_Minus) + LHS = dyn_cast(UO->getSubExpr()); + if (!LHS) + return; + } else { + return; + } + } + + const auto *RHS = dyn_cast(CO->getFalseExpr()); + if (!RHS) { + if (auto *UO = dyn_cast(CO->getFalseExpr())) { + if (UO->getOpcode() == UO_Minus) + RHS = dyn_cast(UO->getSubExpr()); + if (!RHS) + return; + } else { + return; + } + } + + if ((LHS->getValue() == 0 || LHS->getValue() == 1) && + (RHS->getValue() == 0 || RHS->getValue() == 1)) + // Do not diagnose common idioms + return; + if (LHS->getValue() != 0 && LHS->getValue() != 0) + S.Diag(ExprLoc, diag::warn_integer_constants_in_conditional_always_true); + } +} + static void CheckImplicitConversion(Sema &S, Expr *E, QualType T, SourceLocation CC, bool *ICContext = nullptr, @@ -11708,6 +11746,9 @@ static void CheckConditionalOperator(Sema &S, ConditionalOperator *E, CheckConditionalOperand(S, E->getTrueExpr(), T, CC, Suspicious); CheckConditionalOperand(S, E->getFalseExpr(), T, CC, Suspicious); + if (T->isBooleanType()) + DiagnoseIntInBoolContext(S, E); + // If -Wconversion would have warned about either of the candidates // for a signedness conversion to the context type... if (!Suspicious) return; diff --git a/clang/test/Sema/warn-integer-constants-in-ternary.c b/clang/test/Sema/warn-integer-constants-in-ternary.c new file mode 100644 index 0000000000000..287b91ecdb723 --- /dev/null +++ b/clang/test/Sema/warn-integer-constants-in-ternary.c @@ -0,0 +1,32 @@ +// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wtautological-constant-compare %s +// RUN: %clang_cc1 -x c -fsyntax-only -verify %s +// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wtautological-constant-compare %s +// RUN: %clang_cc1 -x c++ -fsyntax-only -verify %s + +#define ONE 1 +#define TWO 2 + +#define TERN(c, l, r) c ? l : r + +#ifdef __cplusplus +typedef bool boolean; +#else +typedef _Bool boolean; +#endif + +void test(boolean a) { + boolean r; + r = a ? (1) : TWO; + r = a ? 3 : TWO; // expected-warning {{converting the result of '?:' with integer constants to a boolean always evaluates to 'true'}} + r = a ? -2 : 0; // expected-warning {{converting the result of '?:' with integer constants to a boolean always evaluates to 'true'}} + r = a ? 3 : -2; // expected-warning {{converting the result of '?:' with integer constants to a boolean always evaluates to 'true'}} + r = a ? 0 : TWO; + r = a ? 3 : ONE; // expected-warning {{converting the result of '?:' with integer constants to a boolean always evaluates to 'true'}} + r = a ? ONE : 0; + r = a ? 0 : -0; + r = a ? 1 : 0; + r = a ? ONE : 0; + r = a ? ONE : ONE; + r = TERN(a, 4, 8); // expected-warning {{converting the result of '?:' with integer constants to a boolean always evaluates to 'true'}} + r = TERN(a, -1, -8); // expected-warning {{converting the result of '?:' with integer constants to a boolean always evaluates to 'true'}} +} diff --git a/clang/test/SemaCXX/constexpr-builtin-bit-cast.cpp b/clang/test/SemaCXX/constexpr-builtin-bit-cast.cpp index d48ee03dccebb..8f4c34744d9f1 100644 --- a/clang/test/SemaCXX/constexpr-builtin-bit-cast.cpp +++ b/clang/test/SemaCXX/constexpr-builtin-bit-cast.cpp @@ -220,7 +220,7 @@ void backtrace() { void test_array_fill() { constexpr unsigned char a[4] = {1, 2}; constexpr unsigned int i = bit_cast(a); - static_assert(i == LITTLE_END ? 0x00000201 : 0x01020000, ""); + static_assert(i == LITTLE_END ? 0x00000201 : 0x01020000, ""); // expected-warning {{converting the result of '?:' with integer constants to a boolean always evaluates to 'true'}} } typedef decltype(nullptr) nullptr_t; diff --git a/clang/test/SemaCXX/cxx2a-explicit-bool.cpp b/clang/test/SemaCXX/cxx2a-explicit-bool.cpp index 1c532cfb59766..56fa76f0a8bd5 100644 --- a/clang/test/SemaCXX/cxx2a-explicit-bool.cpp +++ b/clang/test/SemaCXX/cxx2a-explicit-bool.cpp @@ -717,3 +717,21 @@ A d2{0, 0}; A d3 = {0.0, 0.0};// expected-error {{explicit deduction guide}} } + +namespace PR42980 { +using size_t = decltype(sizeof(0)); + +struct Str {// expected-note+ {{candidate constructor}} + template + explicit(N > 7)// expected-note {{resolved to true}} + Str(char const (&str)[N]); +}; + +template +Str::Str(char const(&str)[N]) { } +// expected-note@-1 {{candidate constructor}} + +Str a = "short"; +Str b = "not so short";// expected-error {{no viable conversion}} + +} \ No newline at end of file diff --git a/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp b/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp index 8a1dd62f057db..18a07eb69d0b9 100644 --- a/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp +++ b/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp @@ -3441,8 +3441,9 @@ MachineSDNode *X86DAGToDAGISel::matchBEXTRFromAndImm(SDNode *Node) { // TODO: Maybe load folding, greater than 32-bit masks, or a guarantee of LICM // hoisting the move immediate would make it worthwhile with a less optimal // BEXTR? - if (!Subtarget->hasTBM() && - !(Subtarget->hasBMI() && Subtarget->hasFastBEXTR())) + bool PreferBEXTR = + Subtarget->hasTBM() || (Subtarget->hasBMI() && Subtarget->hasFastBEXTR()); + if (!PreferBEXTR && !Subtarget->hasBMI2()) return nullptr; // Must have a shift right. @@ -3481,23 +3482,50 @@ MachineSDNode *X86DAGToDAGISel::matchBEXTRFromAndImm(SDNode *Node) { if (Shift + MaskSize > NVT.getSizeInBits()) return nullptr; - SDValue New = CurDAG->getTargetConstant(Shift | (MaskSize << 8), dl, NVT); - unsigned ROpc = NVT == MVT::i64 ? X86::BEXTRI64ri : X86::BEXTRI32ri; - unsigned MOpc = NVT == MVT::i64 ? X86::BEXTRI64mi : X86::BEXTRI32mi; + // BZHI, if available, is always fast, unlike BEXTR. But even if we decide + // that we can't use BEXTR, it is only worthwhile using BZHI if the mask + // does not fit into 32 bits. Load folding is not a sufficient reason. + if (!PreferBEXTR && MaskSize <= 32) + return nullptr; - // BMI requires the immediate to placed in a register. - if (!Subtarget->hasTBM()) { - ROpc = NVT == MVT::i64 ? X86::BEXTR64rr : X86::BEXTR32rr; - MOpc = NVT == MVT::i64 ? X86::BEXTR64rm : X86::BEXTR32rm; + SDValue Control; + unsigned ROpc, MOpc; + + if (!PreferBEXTR) { + assert(Subtarget->hasBMI2() && "We must have BMI2's BZHI then."); + // If we can't make use of BEXTR then we can't fuse shift+mask stages. + // Let's perform the mask first, and apply shift later. Note that we need to + // widen the mask to account for the fact that we'll apply shift afterwards! + Control = CurDAG->getTargetConstant(Shift + MaskSize, dl, NVT); + ROpc = NVT == MVT::i64 ? X86::BZHI64rr : X86::BZHI32rr; + MOpc = NVT == MVT::i64 ? X86::BZHI64rm : X86::BZHI32rm; unsigned NewOpc = NVT == MVT::i64 ? X86::MOV32ri64 : X86::MOV32ri; - New = SDValue(CurDAG->getMachineNode(NewOpc, dl, NVT, New), 0); + Control = SDValue(CurDAG->getMachineNode(NewOpc, dl, NVT, Control), 0); + } else { + // The 'control' of BEXTR has the pattern of: + // [15...8 bit][ 7...0 bit] location + // [ bit count][ shift] name + // I.e. 0b000000011'00000001 means (x >> 0b1) & 0b11 + Control = CurDAG->getTargetConstant(Shift | (MaskSize << 8), dl, NVT); + if (Subtarget->hasTBM()) { + ROpc = NVT == MVT::i64 ? X86::BEXTRI64ri : X86::BEXTRI32ri; + MOpc = NVT == MVT::i64 ? X86::BEXTRI64mi : X86::BEXTRI32mi; + } else { + assert(Subtarget->hasBMI() && "We must have BMI1's BEXTR then."); + // BMI requires the immediate to placed in a register. + ROpc = NVT == MVT::i64 ? X86::BEXTR64rr : X86::BEXTR32rr; + MOpc = NVT == MVT::i64 ? X86::BEXTR64rm : X86::BEXTR32rm; + unsigned NewOpc = NVT == MVT::i64 ? X86::MOV32ri64 : X86::MOV32ri; + Control = SDValue(CurDAG->getMachineNode(NewOpc, dl, NVT, Control), 0); + } } MachineSDNode *NewNode; SDValue Input = N0->getOperand(0); SDValue Tmp0, Tmp1, Tmp2, Tmp3, Tmp4; if (tryFoldLoad(Node, N0.getNode(), Input, Tmp0, Tmp1, Tmp2, Tmp3, Tmp4)) { - SDValue Ops[] = { Tmp0, Tmp1, Tmp2, Tmp3, Tmp4, New, Input.getOperand(0) }; + SDValue Ops[] = { + Tmp0, Tmp1, Tmp2, Tmp3, Tmp4, Control, Input.getOperand(0)}; SDVTList VTs = CurDAG->getVTList(NVT, MVT::i32, MVT::Other); NewNode = CurDAG->getMachineNode(MOpc, dl, VTs, Ops); // Update the chain. @@ -3505,7 +3533,15 @@ MachineSDNode *X86DAGToDAGISel::matchBEXTRFromAndImm(SDNode *Node) { // Record the mem-refs CurDAG->setNodeMemRefs(NewNode, {cast(Input)->getMemOperand()}); } else { - NewNode = CurDAG->getMachineNode(ROpc, dl, NVT, MVT::i32, Input, New); + NewNode = CurDAG->getMachineNode(ROpc, dl, NVT, MVT::i32, Input, Control); + } + + if (!PreferBEXTR) { + // We still need to apply the shift. + SDValue ShAmt = CurDAG->getTargetConstant(Shift, dl, NVT); + unsigned NewOpc = NVT == MVT::i64 ? X86::SHR64ri : X86::SHR32ri; + NewNode = + CurDAG->getMachineNode(NewOpc, dl, NVT, SDValue(NewNode, 0), ShAmt); } return NewNode; diff --git a/llvm/test/CodeGen/X86/bmi-x86_64.ll b/llvm/test/CodeGen/X86/bmi-x86_64.ll index 2932b7d757a67..7646fd6bbb068 100644 --- a/llvm/test/CodeGen/X86/bmi-x86_64.ll +++ b/llvm/test/CodeGen/X86/bmi-x86_64.ll @@ -86,9 +86,9 @@ define i64 @bextr64d(i64 %a) { ; ; BMI2-SLOW-LABEL: bextr64d: ; BMI2-SLOW: # %bb.0: # %entry -; BMI2-SLOW-NEXT: shrq $2, %rdi -; BMI2-SLOW-NEXT: movb $33, %al +; BMI2-SLOW-NEXT: movl $35, %eax ; BMI2-SLOW-NEXT: bzhiq %rax, %rdi, %rax +; BMI2-SLOW-NEXT: shrq $2, %rax ; BMI2-SLOW-NEXT: retq ; ; BEXTR-FAST-LABEL: bextr64d: @@ -113,10 +113,9 @@ define i64 @bextr64d_load(i64* %aptr) { ; ; BMI2-SLOW-LABEL: bextr64d_load: ; BMI2-SLOW: # %bb.0: # %entry -; BMI2-SLOW-NEXT: movq (%rdi), %rax +; BMI2-SLOW-NEXT: movl $35, %eax +; BMI2-SLOW-NEXT: bzhiq %rax, (%rdi), %rax ; BMI2-SLOW-NEXT: shrq $2, %rax -; BMI2-SLOW-NEXT: movb $33, %cl -; BMI2-SLOW-NEXT: bzhiq %rcx, %rax, %rax ; BMI2-SLOW-NEXT: retq ; ; BEXTR-FAST-LABEL: bextr64d_load: