Skip to content

Commit

Permalink
Reapply "[clang][bytecode] Fix some shift edge cases (#119895)"
Browse files Browse the repository at this point in the history 
This reverts commit a6636ce.

This original commit failed on hosts with signed wchar_t. Care for
this in the test.
  • Loading branch information
@tbaederr
tbaederr committed Dec 16, 2024
1 parent a246454 commit 2503a66
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 51 deletions.
7 changes: 5 additions & 2 deletions clang/lib/AST/ByteCode/Integral.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,10 @@ template <unsigned Bits, bool Signed> class Integral final {
unsigned countLeadingZeros() const {
if constexpr (!Signed)
return llvm::countl_zero<ReprT>(V);
llvm_unreachable("Don't call countLeadingZeros() on signed types.");
if (isPositive())
return llvm::countl_zero<typename AsUnsigned::ReprT>(
static_cast<typename AsUnsigned::ReprT>(V));
llvm_unreachable("Don't call countLeadingZeros() on negative values.");
}

Integral truncate(unsigned TruncBits) const {
Expand Down Expand Up @@ -210,7 +213,7 @@ template <unsigned Bits, bool Signed> class Integral final {
return Integral(Value.V);
}

static Integral zero() { return from(0); }
static Integral zero(unsigned BitWidth = 0) { return from(0); }

template <typename T> static Integral from(T Value, unsigned NumBits) {
return Integral(Value);
Expand Down
68 changes: 35 additions & 33 deletions clang/lib/AST/ByteCode/Interp.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2511,50 +2511,52 @@ 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 (RHS > RT::from(Bits - 1, RHS.bitWidth()))
LT::AsUnsigned::shiftLeft(LT::AsUnsigned::from(LHS),
LT::AsUnsigned::from(Bits - 1), Bits, &R);
else
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.
LT::AsUnsigned::shiftLeft(LT::AsUnsigned::from(LHS),
LT::AsUnsigned::from(RHS, Bits), Bits, &R);
}
} else {
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);
// 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);
}
}

Expand Down
47 changes: 31 additions & 16 deletions clang/test/AST/ByteCode/shifts.cpp
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -std=c++20 -verify=expected,all %s
// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -std=c++17 -verify=cxx17,all %s
// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -std=c++17 -verify=cxx17,all -triple armv8 %s
// RUN: %clang_cc1 -std=c++20 -verify=ref,all %s
// RUN: %clang_cc1 -std=c++17 -verify=ref-cxx17,all %s
// RUN: %clang_cc1 -std=c++17 -verify=ref-cxx17,all -triple armv8 %s

#define INT_MIN (~__INT_MAX__)

Expand All @@ -21,27 +23,15 @@ namespace shifts {
c = 1 << 0;
c = 1 << -0;
c = 1 >> -0;
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; // all-warning {{shift count is negative}} \
// all-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; // 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; // 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}} \
// cxx17-warning {{shift count >= width of type}} \
// ref-warning {{shift count >= width of type}} \
Expand Down Expand Up @@ -212,3 +202,28 @@ 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
# if !defined(__WCHAR_UNSIGNED__)
static_assert(((wchar_t)-1U >> 31) == -1);
# else
static_assert(((wchar_t)-1U >> 31) == 1);
# endif
#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}}

0 comments on commit 2503a66

Please sign in to comment.