Author: Timm Bäder Date: 2024-12-14T06:28:12+01:00 New Revision: a6636ce4d124176856c3913d4bf6c3ceff1f5a1f
URL: https://github.com/llvm/llvm-project/commit/a6636ce4d124176856c3913d4bf6c3ceff1f5a1f DIFF: https://github.com/llvm/llvm-project/commit/a6636ce4d124176856c3913d4bf6c3ceff1f5a1f.diff LOG: Revert "[clang][bytecode] Fix some shift edge cases (#119895)" This reverts commit 49c2207f21c0922aedb6c70471f8ea068977eb30. This breaks on big-endian, again: https://lab.llvm.org/buildbot/#/builders/154/builds/9018 Added: Modified: clang/lib/AST/ByteCode/Integral.h clang/lib/AST/ByteCode/Interp.h clang/test/AST/ByteCode/shifts.cpp Removed: ################################################################################ diff --git a/clang/lib/AST/ByteCode/Integral.h b/clang/lib/AST/ByteCode/Integral.h index 13fdb5369f2b7a..26585799e5eadc 100644 --- a/clang/lib/AST/ByteCode/Integral.h +++ b/clang/lib/AST/ByteCode/Integral.h @@ -179,10 +179,7 @@ template <unsigned Bits, bool Signed> class Integral final { unsigned countLeadingZeros() const { if constexpr (!Signed) return llvm::countl_zero<ReprT>(V); - if (isPositive()) - return llvm::countl_zero<typename AsUnsigned::ReprT>( - static_cast<typename AsUnsigned::ReprT>(V)); - llvm_unreachable("Don't call countLeadingZeros() on negative values."); + llvm_unreachable("Don't call countLeadingZeros() on signed types."); } Integral truncate(unsigned TruncBits) const { @@ -213,7 +210,7 @@ template <unsigned Bits, bool Signed> class Integral final { return Integral(Value.V); } - static Integral zero(unsigned BitWidth = 0) { return from(0); } + static Integral zero() { return from(0); } template <typename T> static Integral from(T Value, unsigned NumBits) { return Integral(Value); diff --git a/clang/lib/AST/ByteCode/Interp.h b/clang/lib/AST/ByteCode/Interp.h index f085f96fdf5391..cdf05e36304acb 100644 --- a/clang/lib/AST/ByteCode/Interp.h +++ b/clang/lib/AST/ByteCode/Interp.h @@ -2511,52 +2511,50 @@ inline bool DoShift(InterpState &S, CodePtr OpPC, LT &LHS, RT &RHS) { S, OpPC, LHS, RHS); } + if constexpr (Dir == ShiftDir::Left) { + if (LHS.isNegative() && !S.getLangOpts().CPlusPlus20) { + // C++11 [expr.shift]p2: A signed left shift must have a non-negative + // operand, and must not overflow the corresponding unsigned type. + // C++2a [expr.shift]p2: E1 << E2 is the unique value congruent to + // E1 x 2^E2 module 2^N. + const SourceInfo &Loc = S.Current->getSource(OpPC); + S.CCEDiag(Loc, diag::note_constexpr_lshift_of_negative) << LHS.toAPSInt(); + if (!S.noteUndefinedBehavior()) + return false; + } + } + if (!CheckShift<Dir>(S, OpPC, LHS, RHS, Bits)) return false; // Limit the shift amount to Bits - 1. If this happened, // it has already been diagnosed by CheckShift() above, // but we still need to handle it. - // Note that we have to be extra careful here since we're doing the shift in - // any case, but we need to adjust the shift amount or the way we do the shift - // for the potential error cases. typename LT::AsUnsigned R; - unsigned MaxShiftAmount = LHS.bitWidth() - 1; if constexpr (Dir == ShiftDir::Left) { - if (Compare(RHS, RT::from(MaxShiftAmount, RHS.bitWidth())) == - ComparisonCategoryResult::Greater) { - if (LHS.isNegative()) - R = LT::AsUnsigned::zero(LHS.bitWidth()); - else { - RHS = RT::from(LHS.countLeadingZeros(), RHS.bitWidth()); - LT::AsUnsigned::shiftLeft(LT::AsUnsigned::from(LHS), - LT::AsUnsigned::from(RHS, Bits), Bits, &R); - } - } else if (LHS.isNegative()) { - if (LHS.isMin()) { - R = LT::AsUnsigned::zero(LHS.bitWidth()); - } else { - // If the LHS is negative, perform the cast and invert the result. - typename LT::AsUnsigned LHSU = LT::AsUnsigned::from(-LHS); - LT::AsUnsigned::shiftLeft(LHSU, LT::AsUnsigned::from(RHS, Bits), Bits, - &R); - R = -R; - } - } else { - // The good case, a simple left shift. + if (RHS > RT::from(Bits - 1, RHS.bitWidth())) + LT::AsUnsigned::shiftLeft(LT::AsUnsigned::from(LHS), + LT::AsUnsigned::from(Bits - 1), Bits, &R); + else LT::AsUnsigned::shiftLeft(LT::AsUnsigned::from(LHS), LT::AsUnsigned::from(RHS, Bits), Bits, &R); - } } else { - // Right shift. - if (Compare(RHS, RT::from(MaxShiftAmount, RHS.bitWidth())) == - ComparisonCategoryResult::Greater) { - R = LT::AsUnsigned::from(-1); - } else { - // Do the shift on potentially signed LT, then convert to unsigned type. - LT A; - LT::shiftRight(LHS, LT::from(RHS, Bits), Bits, &A); - R = LT::AsUnsigned::from(A); + if (RHS > RT::from(Bits - 1, RHS.bitWidth())) + LT::AsUnsigned::shiftRight(LT::AsUnsigned::from(LHS), + LT::AsUnsigned::from(Bits - 1), Bits, &R); + else + LT::AsUnsigned::shiftRight(LT::AsUnsigned::from(LHS), + LT::AsUnsigned::from(RHS, Bits), Bits, &R); + } + + // We did the shift above as unsigned. Restore the sign bit if we need to. + if constexpr (Dir == ShiftDir::Right) { + if (LHS.isSigned() && LHS.isNegative()) { + typename LT::AsUnsigned SignBit; + LT::AsUnsigned::shiftLeft(LT::AsUnsigned::from(1, Bits), + LT::AsUnsigned::from(Bits - 1, Bits), Bits, + &SignBit); + LT::AsUnsigned::bitOr(R, SignBit, Bits, &R); } } diff --git a/clang/test/AST/ByteCode/shifts.cpp b/clang/test/AST/ByteCode/shifts.cpp index 493f8b78204259..0b3383731c6774 100644 --- a/clang/test/AST/ByteCode/shifts.cpp +++ b/clang/test/AST/ByteCode/shifts.cpp @@ -21,15 +21,27 @@ namespace shifts { c = 1 << 0; c = 1 << -0; c = 1 >> -0; - c = 1 << -1; // all-warning {{shift count is negative}} \ - // all-note {{negative shift count -1}} + c = 1 << -1; // expected-warning {{shift count is negative}} \ + // expected-note {{negative shift count -1}} \ + // cxx17-note {{negative shift count -1}} \ + // cxx17-warning {{shift count is negative}} \ + // ref-warning {{shift count is negative}} \ + // ref-note {{negative shift count -1}} \ + // ref-cxx17-warning {{shift count is negative}} \ + // ref-cxx17-note {{negative shift count -1}} c = 1 >> -1; // expected-warning {{shift count is negative}} \ // cxx17-warning {{shift count is negative}} \ // ref-warning {{shift count is negative}} \ // ref-cxx17-warning {{shift count is negative}} - c = 1 << (unsigned)-1; // all-warning {{shift count >= width of type}} \ - // all-warning {{implicit conversion from 'int' to 'char' changes value from -2147483648 to 0}} + c = 1 << (unsigned)-1; // expected-warning {{shift count >= width of type}} \ + // expected-warning {{implicit conversion from 'int' to 'char' changes value from -2147483648 to 0}} \ + // cxx17-warning {{shift count >= width of type}} \ + // cxx17-warning {{implicit conversion from 'int' to 'char' changes value from -2147483648 to 0}} \ + // ref-warning {{shift count >= width of type}} \ + // ref-warning {{implicit conversion from 'int' to 'char' changes value from -2147483648 to 0}} \ + // ref-cxx17-warning {{shift count >= width of type}} \ + // ref-cxx17-warning {{implicit conversion from 'int' to 'char' changes value from -2147483648 to 0}} c = 1 >> (unsigned)-1; // expected-warning {{shift count >= width of type}} \ // cxx17-warning {{shift count >= width of type}} \ // ref-warning {{shift count >= width of type}} \ @@ -200,24 +212,3 @@ enum shiftof { X3 = (1<<32) // all-error {{expression is not an integral constant expression}} \ // all-note {{shift count 32 >= width of type 'int'}} }; - -#if __WCHAR_WIDTH__ == 32 -static_assert(((wchar_t)-1U >> 31) == -1); -#endif - -#if __INT_WIDTH__ == 32 -static_assert(((int)-1U >> 32) == -1); // all-error {{not an integral constant expression}} \ - // all-note {{shift count 32 >= width of type 'int' (32 bits)}} -#endif - -static_assert((-4 << 32) == 0); // all-error {{not an integral constant expression}} \ - // all-note {{shift count}} - -static_assert((-4 << 1) == -8); // ref-cxx17-error {{not an integral constant expression}} \ - // ref-cxx17-note {{left shift of negative value -4}} \ - // cxx17-error {{not an integral constant expression}} \ - // cxx17-note {{left shift of negative value -4}} -static_assert((-4 << 31) == 0); // ref-cxx17-error {{not an integral constant expression}} \ - // ref-cxx17-note {{left shift of negative value -4}} \ - // cxx17-error {{not an integral constant expression}} \ - // cxx17-note {{left shift of negative value -4}} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
