[clang] de02994 - [clang][Interp] Handle negative shift amounts correctly

Timm Bäder via cfe-commits cfe-commits at lists.llvm.org
Sat Jul 13 22:43:17 PDT 2024


Author: Timm Bäder
Date: 2024-07-14T07:28:02+02:00
New Revision: de029943cc5ad0028f16e6ecaffa03e32ffd1a6f

URL: https://github.com/llvm/llvm-project/commit/de029943cc5ad0028f16e6ecaffa03e32ffd1a6f
DIFF: https://github.com/llvm/llvm-project/commit/de029943cc5ad0028f16e6ecaffa03e32ffd1a6f.diff

LOG: [clang][Interp] Handle negative shift amounts correctly

We need to invert them and use the opposite shift.

Added: 
    

Modified: 
    clang/lib/AST/Interp/Interp.h
    clang/test/AST/Interp/shifts.cpp
    clang/test/Sema/shift-count-negative.c

Removed: 
    


################################################################################
diff  --git a/clang/lib/AST/Interp/Interp.h b/clang/lib/AST/Interp/Interp.h
index 95f89990a7a18..8502b7ca136ee 100644
--- a/clang/lib/AST/Interp/Interp.h
+++ b/clang/lib/AST/Interp/Interp.h
@@ -2209,13 +2209,10 @@ inline bool RVOPtr(InterpState &S, CodePtr OpPC) {
 //===----------------------------------------------------------------------===//
 // Shr, Shl
 //===----------------------------------------------------------------------===//
+enum class ShiftDir { Left, Right };
 
-template <PrimType NameL, PrimType NameR>
-inline bool Shr(InterpState &S, CodePtr OpPC) {
-  using LT = typename PrimConv<NameL>::T;
-  using RT = typename PrimConv<NameR>::T;
-  auto RHS = S.Stk.pop<RT>();
-  const auto &LHS = S.Stk.pop<LT>();
+template <class LT, class RT, ShiftDir Dir>
+inline bool DoShift(InterpState &S, CodePtr OpPC, LT &LHS, RT &RHS) {
   const unsigned Bits = LHS.bitWidth();
 
   // OpenCL 6.3j: shift values are effectively % word size of LHS.
@@ -2223,6 +2220,20 @@ inline bool Shr(InterpState &S, CodePtr OpPC) {
     RT::bitAnd(RHS, RT::from(LHS.bitWidth() - 1, RHS.bitWidth()),
                RHS.bitWidth(), &RHS);
 
+  if (RHS.isNegative()) {
+    // During constant-folding, a negative shift is an opposite shift. Such a
+    // shift is not a constant expression.
+    const SourceInfo &Loc = S.Current->getSource(OpPC);
+    S.CCEDiag(Loc, diag::note_constexpr_negative_shift) << RHS.toAPSInt();
+    if (S.getLangOpts().CPlusPlus11 && S.getEvalStatus().Diag &&
+        !S.getEvalStatus().Diag->empty())
+      return false;
+    RHS = -RHS;
+    return DoShift < LT, RT,
+           Dir == ShiftDir::Left ? ShiftDir::Right
+                                 : ShiftDir::Left > (S, OpPC, LHS, RHS);
+  }
+
   if (!CheckShift(S, OpPC, LHS, RHS, Bits))
     return false;
 
@@ -2230,45 +2241,44 @@ inline bool Shr(InterpState &S, CodePtr OpPC) {
   // it has already been diagnosed by CheckShift() above,
   // but we still need to handle it.
   typename LT::AsUnsigned R;
-  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);
+  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
+      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);
+  }
+
   S.Stk.push<LT>(LT::from(R));
   return true;
 }
 
 template <PrimType NameL, PrimType NameR>
-inline bool Shl(InterpState &S, CodePtr OpPC) {
+inline bool Shr(InterpState &S, CodePtr OpPC) {
   using LT = typename PrimConv<NameL>::T;
   using RT = typename PrimConv<NameR>::T;
   auto RHS = S.Stk.pop<RT>();
-  const auto &LHS = S.Stk.pop<LT>();
-  const unsigned Bits = LHS.bitWidth();
-
-  // OpenCL 6.3j: shift values are effectively % word size of LHS.
-  if (S.getLangOpts().OpenCL)
-    RT::bitAnd(RHS, RT::from(LHS.bitWidth() - 1, RHS.bitWidth()),
-               RHS.bitWidth(), &RHS);
+  auto LHS = S.Stk.pop<LT>();
 
-  if (!CheckShift(S, OpPC, LHS, RHS, Bits))
-    return false;
+  return DoShift<LT, RT, ShiftDir::Right>(S, OpPC, LHS, RHS);
+}
 
-  // 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.
-  typename LT::AsUnsigned R;
-  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);
+template <PrimType NameL, PrimType NameR>
+inline bool Shl(InterpState &S, CodePtr OpPC) {
+  using LT = typename PrimConv<NameL>::T;
+  using RT = typename PrimConv<NameR>::T;
+  auto RHS = S.Stk.pop<RT>();
+  auto LHS = S.Stk.pop<LT>();
 
-  S.Stk.push<LT>(LT::from(R));
-  return true;
+  return DoShift<LT, RT, ShiftDir::Left>(S, OpPC, LHS, RHS);
 }
 
 //===----------------------------------------------------------------------===//

diff  --git a/clang/test/AST/Interp/shifts.cpp b/clang/test/AST/Interp/shifts.cpp
index 76047d0f752d5..ce17fabc7833f 100644
--- a/clang/test/AST/Interp/shifts.cpp
+++ b/clang/test/AST/Interp/shifts.cpp
@@ -1,7 +1,7 @@
-// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -std=c++20 -verify %s
-// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -std=c++17 -verify=cxx17 %s
-// RUN: %clang_cc1 -std=c++20 -verify=ref %s
-// RUN: %clang_cc1 -std=c++17 -verify=ref-cxx17 %s
+// 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 -std=c++20 -verify=ref,all %s
+// RUN: %clang_cc1 -std=c++17 -verify=ref-cxx17,all %s
 
 #define INT_MIN (~__INT_MAX__)
 
@@ -198,3 +198,8 @@ namespace LongInt {
   }
   static_assert(f() == 1, "");
 };
+
+enum shiftof {
+    X = (1<<-29) // all-error {{expression is not an integral constant expression}} \
+                 // all-note {{negative shift count -29}}
+};

diff  --git a/clang/test/Sema/shift-count-negative.c b/clang/test/Sema/shift-count-negative.c
index 97f85feed52c0..84c7625187a68 100644
--- a/clang/test/Sema/shift-count-negative.c
+++ b/clang/test/Sema/shift-count-negative.c
@@ -1,6 +1,9 @@
 // RUN: %clang_cc1 -x c -fsyntax-only -verify=expected,c -pedantic %s
 // RUN: %clang_cc1 -x c++ -fsyntax-only -verify=expected,cpp %s
 
+// RUN: %clang_cc1 -x c -fsyntax-only -verify=expected,c -pedantic %s -fexperimental-new-constant-interpreter
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify=expected,cpp %s -fexperimental-new-constant-interpreter
+
 enum shiftof {
     X = (1<<-29) // c-warning {{expression is not an integer constant expression; folding it to a constant is a GNU extension}}
                  // cpp-error at -1 {{expression is not an integral constant expression}}


        


More information about the cfe-commits mailing list