Skip to content

Commit a6636ce

Browse files
tbaederrtbaederr
committed
Revert "[clang][bytecode] Fix some shift edge cases (#119895)"
This reverts commit 49c2207. This breaks on big-endian, again: https://lab.llvm.org/buildbot/#/builders/154/builds/9018
1 parent 49c2207 commit a6636ce

File tree

3 files changed

+51
-65
lines changed

3 files changed

+51
-65
lines changed

‎clang/lib/AST/ByteCode/Integral.h‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -179,10 +179,7 @@ template <unsigned Bits, bool Signed> class Integral final {
179179
unsigned countLeadingZeros() const {
180180
if constexpr (!Signed)
181181
return llvm::countl_zero<ReprT>(V);
182-
if (isPositive())
183-
return llvm::countl_zero<typename AsUnsigned::ReprT>(
184-
static_cast<typename AsUnsigned::ReprT>(V));
185-
llvm_unreachable("Don't call countLeadingZeros() on negative values.");
182+
llvm_unreachable("Don't call countLeadingZeros() on signed types.");
186183
}
187184

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

216-
static Integral zero(unsigned BitWidth = 0) { return from(0); }
213+
static Integral zero() { return from(0); }
217214

218215
template <typename T> static Integral from(T Value, unsigned NumBits) {
219216
return Integral(Value);

‎clang/lib/AST/ByteCode/Interp.h‎

Lines changed: 33 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -2511,52 +2511,50 @@ inline bool DoShift(InterpState &S, CodePtr OpPC, LT &LHS, RT &RHS) {
25112511
S, OpPC, LHS, RHS);
25122512
}
25132513

2514+
if constexpr (Dir == ShiftDir::Left) {
2515+
if (LHS.isNegative() && !S.getLangOpts().CPlusPlus20) {
2516+
// C++11 [expr.shift]p2: A signed left shift must have a non-negative
2517+
// operand, and must not overflow the corresponding unsigned type.
2518+
// C++2a [expr.shift]p2: E1 << E2 is the unique value congruent to
2519+
// E1 x 2^E2 module 2^N.
2520+
const SourceInfo &Loc = S.Current->getSource(OpPC);
2521+
S.CCEDiag(Loc, diag::note_constexpr_lshift_of_negative) << LHS.toAPSInt();
2522+
if (!S.noteUndefinedBehavior())
2523+
return false;
2524+
}
2525+
}
2526+
25142527
if (!CheckShift<Dir>(S, OpPC, LHS, RHS, Bits))
25152528
return false;
25162529

25172530
// Limit the shift amount to Bits - 1. If this happened,
25182531
// it has already been diagnosed by CheckShift() above,
25192532
// but we still need to handle it.
2520-
// Note that we have to be extra careful here since we're doing the shift in
2521-
// any case, but we need to adjust the shift amount or the way we do the shift
2522-
// for the potential error cases.
25232533
typename LT::AsUnsigned R;
2524-
unsigned MaxShiftAmount = LHS.bitWidth() - 1;
25252534
if constexpr (Dir == ShiftDir::Left) {
2526-
if (Compare(RHS, RT::from(MaxShiftAmount, RHS.bitWidth())) ==
2527-
ComparisonCategoryResult::Greater) {
2528-
if (LHS.isNegative())
2529-
R = LT::AsUnsigned::zero(LHS.bitWidth());
2530-
else {
2531-
RHS = RT::from(LHS.countLeadingZeros(), RHS.bitWidth());
2532-
LT::AsUnsigned::shiftLeft(LT::AsUnsigned::from(LHS),
2533-
LT::AsUnsigned::from(RHS, Bits), Bits, &R);
2534-
}
2535-
} else if (LHS.isNegative()) {
2536-
if (LHS.isMin()) {
2537-
R = LT::AsUnsigned::zero(LHS.bitWidth());
2538-
} else {
2539-
// If the LHS is negative, perform the cast and invert the result.
2540-
typename LT::AsUnsigned LHSU = LT::AsUnsigned::from(-LHS);
2541-
LT::AsUnsigned::shiftLeft(LHSU, LT::AsUnsigned::from(RHS, Bits), Bits,
2542-
&R);
2543-
R = -R;
2544-
}
2545-
} else {
2546-
// The good case, a simple left shift.
2535+
if (RHS > RT::from(Bits - 1, RHS.bitWidth()))
2536+
LT::AsUnsigned::shiftLeft(LT::AsUnsigned::from(LHS),
2537+
LT::AsUnsigned::from(Bits - 1), Bits, &R);
2538+
else
25472539
LT::AsUnsigned::shiftLeft(LT::AsUnsigned::from(LHS),
25482540
LT::AsUnsigned::from(RHS, Bits), Bits, &R);
2549-
}
25502541
} else {
2551-
// Right shift.
2552-
if (Compare(RHS, RT::from(MaxShiftAmount, RHS.bitWidth())) ==
2553-
ComparisonCategoryResult::Greater) {
2554-
R = LT::AsUnsigned::from(-1);
2555-
} else {
2556-
// Do the shift on potentially signed LT, then convert to unsigned type.
2557-
LT A;
2558-
LT::shiftRight(LHS, LT::from(RHS, Bits), Bits, &A);
2559-
R = LT::AsUnsigned::from(A);
2542+
if (RHS > RT::from(Bits - 1, RHS.bitWidth()))
2543+
LT::AsUnsigned::shiftRight(LT::AsUnsigned::from(LHS),
2544+
LT::AsUnsigned::from(Bits - 1), Bits, &R);
2545+
else
2546+
LT::AsUnsigned::shiftRight(LT::AsUnsigned::from(LHS),
2547+
LT::AsUnsigned::from(RHS, Bits), Bits, &R);
2548+
}
2549+
2550+
// We did the shift above as unsigned. Restore the sign bit if we need to.
2551+
if constexpr (Dir == ShiftDir::Right) {
2552+
if (LHS.isSigned() && LHS.isNegative()) {
2553+
typename LT::AsUnsigned SignBit;
2554+
LT::AsUnsigned::shiftLeft(LT::AsUnsigned::from(1, Bits),
2555+
LT::AsUnsigned::from(Bits - 1, Bits), Bits,
2556+
&SignBit);
2557+
LT::AsUnsigned::bitOr(R, SignBit, Bits, &R);
25602558
}
25612559
}
25622560

‎clang/test/AST/ByteCode/shifts.cpp‎

Lines changed: 16 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -21,15 +21,27 @@ namespace shifts {
2121
c = 1 << 0;
2222
c = 1 << -0;
2323
c = 1 >> -0;
24-
c = 1 << -1; // all-warning {{shift count is negative}} \
25-
// all-note {{negative shift count -1}}
24+
c = 1 << -1; // expected-warning {{shift count is negative}} \
25+
// expected-note {{negative shift count -1}} \
26+
// cxx17-note {{negative shift count -1}} \
27+
// cxx17-warning {{shift count is negative}} \
28+
// ref-warning {{shift count is negative}} \
29+
// ref-note {{negative shift count -1}} \
30+
// ref-cxx17-warning {{shift count is negative}} \
31+
// ref-cxx17-note {{negative shift count -1}}
2632

2733
c = 1 >> -1; // expected-warning {{shift count is negative}} \
2834
// cxx17-warning {{shift count is negative}} \
2935
// ref-warning {{shift count is negative}} \
3036
// ref-cxx17-warning {{shift count is negative}}
31-
c = 1 << (unsigned)-1; // all-warning {{shift count >= width of type}} \
32-
// all-warning {{implicit conversion from 'int' to 'char' changes value from -2147483648 to 0}}
37+
c = 1 << (unsigned)-1; // expected-warning {{shift count >= width of type}} \
38+
// expected-warning {{implicit conversion from 'int' to 'char' changes value from -2147483648 to 0}} \
39+
// cxx17-warning {{shift count >= width of type}} \
40+
// cxx17-warning {{implicit conversion from 'int' to 'char' changes value from -2147483648 to 0}} \
41+
// ref-warning {{shift count >= width of type}} \
42+
// ref-warning {{implicit conversion from 'int' to 'char' changes value from -2147483648 to 0}} \
43+
// ref-cxx17-warning {{shift count >= width of type}} \
44+
// ref-cxx17-warning {{implicit conversion from 'int' to 'char' changes value from -2147483648 to 0}}
3345
c = 1 >> (unsigned)-1; // expected-warning {{shift count >= width of type}} \
3446
// cxx17-warning {{shift count >= width of type}} \
3547
// ref-warning {{shift count >= width of type}} \
@@ -200,24 +212,3 @@ enum shiftof {
200212
X3 = (1<<32) // all-error {{expression is not an integral constant expression}} \
201213
// all-note {{shift count 32 >= width of type 'int'}}
202214
};
203-
204-
#if __WCHAR_WIDTH__ == 32
205-
static_assert(((wchar_t)-1U >> 31) == -1);
206-
#endif
207-
208-
#if __INT_WIDTH__ == 32
209-
static_assert(((int)-1U >> 32) == -1); // all-error {{not an integral constant expression}} \
210-
// all-note {{shift count 32 >= width of type 'int' (32 bits)}}
211-
#endif
212-
213-
static_assert((-4 << 32) == 0); // all-error {{not an integral constant expression}} \
214-
// all-note {{shift count}}
215-
216-
static_assert((-4 << 1) == -8); // ref-cxx17-error {{not an integral constant expression}} \
217-
// ref-cxx17-note {{left shift of negative value -4}} \
218-
// cxx17-error {{not an integral constant expression}} \
219-
// cxx17-note {{left shift of negative value -4}}
220-
static_assert((-4 << 31) == 0); // ref-cxx17-error {{not an integral constant expression}} \
221-
// ref-cxx17-note {{left shift of negative value -4}} \
222-
// cxx17-error {{not an integral constant expression}} \
223-
// cxx17-note {{left shift of negative value -4}}

0 commit comments

Comments
 (0)