[clang] a6636ce - Revert "[clang][bytecode] Fix some shift edge cases (#119895)"
Timm Bäder via cfe-commits
cfe-commits at lists.llvm.org
Fri Dec 13 21:28:40 PST 2024
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}}
More information about the cfe-commits
mailing list