Skip to content

Commit 2503a66

Browse files
tbaederrtbaederr
committed
Reapply "[clang][bytecode] Fix some shift edge cases (#119895)"
This reverts commit a6636ce. This original commit failed on hosts with signed wchar_t. Care for this in the test.
1 parent a246454 commit 2503a66

File tree

3 files changed

+71
-51
lines changed

3 files changed

+71
-51
lines changed

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,10 @@ template <unsigned Bits, bool Signed> class Integral final {
179179
unsigned countLeadingZeros() const {
180180
if constexpr (!Signed)
181181
return llvm::countl_zero<ReprT>(V);
182-
llvm_unreachable("Don't call countLeadingZeros() on signed types.");
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.");
183186
}
184187

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

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

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

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

Lines changed: 35 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -2511,50 +2511,52 @@ 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-
25272514
if (!CheckShift<Dir>(S, OpPC, LHS, RHS, Bits))
25282515
return false;
25292516

25302517
// Limit the shift amount to Bits - 1. If this happened,
25312518
// it has already been diagnosed by CheckShift() above,
25322519
// 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.
25332523
typename LT::AsUnsigned R;
2524+
unsigned MaxShiftAmount = LHS.bitWidth() - 1;
25342525
if constexpr (Dir == ShiftDir::Left) {
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
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.
25392547
LT::AsUnsigned::shiftLeft(LT::AsUnsigned::from(LHS),
25402548
LT::AsUnsigned::from(RHS, Bits), Bits, &R);
2549+
}
25412550
} else {
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);
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);
25582560
}
25592561
}
25602562

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

Lines changed: 31 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -std=c++20 -verify=expected,all %s
22
// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -std=c++17 -verify=cxx17,all %s
3+
// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -std=c++17 -verify=cxx17,all -triple armv8 %s
34
// RUN: %clang_cc1 -std=c++20 -verify=ref,all %s
45
// RUN: %clang_cc1 -std=c++17 -verify=ref-cxx17,all %s
6+
// RUN: %clang_cc1 -std=c++17 -verify=ref-cxx17,all -triple armv8 %s
57

68
#define INT_MIN (~__INT_MAX__)
79

@@ -21,27 +23,15 @@ namespace shifts {
2123
c = 1 << 0;
2224
c = 1 << -0;
2325
c = 1 >> -0;
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}}
26+
c = 1 << -1; // all-warning {{shift count is negative}} \
27+
// all-note {{negative shift count -1}}
3228

3329
c = 1 >> -1; // expected-warning {{shift count is negative}} \
3430
// cxx17-warning {{shift count is negative}} \
3531
// ref-warning {{shift count is negative}} \
3632
// ref-cxx17-warning {{shift count is negative}}
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}}
33+
c = 1 << (unsigned)-1; // all-warning {{shift count >= width of type}} \
34+
// all-warning {{implicit conversion from 'int' to 'char' changes value from -2147483648 to 0}}
4535
c = 1 >> (unsigned)-1; // expected-warning {{shift count >= width of type}} \
4636
// cxx17-warning {{shift count >= width of type}} \
4737
// ref-warning {{shift count >= width of type}} \
@@ -212,3 +202,28 @@ enum shiftof {
212202
X3 = (1<<32) // all-error {{expression is not an integral constant expression}} \
213203
// all-note {{shift count 32 >= width of type 'int'}}
214204
};
205+
206+
#if __WCHAR_WIDTH__ == 32
207+
# if !defined(__WCHAR_UNSIGNED__)
208+
static_assert(((wchar_t)-1U >> 31) == -1);
209+
# else
210+
static_assert(((wchar_t)-1U >> 31) == 1);
211+
# endif
212+
#endif
213+
214+
#if __INT_WIDTH__ == 32
215+
static_assert(((int)-1U >> 32) == -1); // all-error {{not an integral constant expression}} \
216+
// all-note {{shift count 32 >= width of type 'int' (32 bits)}}
217+
#endif
218+
219+
static_assert((-4 << 32) == 0); // all-error {{not an integral constant expression}} \
220+
// all-note {{shift count}}
221+
222+
static_assert((-4 << 1) == -8); // ref-cxx17-error {{not an integral constant expression}} \
223+
// ref-cxx17-note {{left shift of negative value -4}} \
224+
// cxx17-error {{not an integral constant expression}} \
225+
// cxx17-note {{left shift of negative value -4}}
226+
static_assert((-4 << 31) == 0); // ref-cxx17-error {{not an integral constant expression}} \
227+
// ref-cxx17-note {{left shift of negative value -4}} \
228+
// cxx17-error {{not an integral constant expression}} \
229+
// cxx17-note {{left shift of negative value -4}}

0 commit comments

Comments
 (0)