[clang][Interp] Make sure we push integers of the correct size

[tbaeder]

Summary

Integers might not be 32 bits wide, so check the TargetInfo for their
size.

[aaron.ballman]

LGTM as far as the changes go, but did have a question about whether we need this for unsigned values as well.

clang/lib/AST/Interp/InterpBuiltin.cpp

36

Do we need the same for unsigned int as well? (I'm thinking in terms of pushing literal values like 0U)

[tbaeder]

clang/lib/AST/Interp/InterpBuiltin.cpp

36

Theoretically yes, but all the ints we're pushing right now are signed AFAICS.

[tbaeder]

This patch is missing the Ret<PT_Sint32> part in InterpBuiltin() in the same file. That needs to change to the correct type as well.

I can do that in a follow-up commit. @aaron.ballman Can you come up with a value I can pass to -triple so I can actually test that this works?

[aaron.ballman]

In D155568#4525488, @tbaeder wrote:

This patch is missing the Ret<PT_Sint32> part in InterpBuiltin() in the same file. That needs to change to the correct type as well.

I can do that in a follow-up commit. @aaron.ballman Can you come up with a value I can pass to -triple so I can actually test that this works?

Oops, sorry on the delayed response, this one slipped under my radar. You should be able to use avr-unknown-unknown: https://godbolt.org/z/4TsPW41d5

[tbaeder]

In D155568#4542764, @aaron.ballman wrote:

In D155568#4525488, @tbaeder wrote:

This patch is missing the Ret<PT_Sint32> part in InterpBuiltin() in the same file. That needs to change to the correct type as well.

I can do that in a follow-up commit. @aaron.ballman Can you come up with a value I can pass to -triple so I can actually test that this works?

Oops, sorry on the delayed response, this one slipped under my radar. You should be able to use avr-unknown-unknown: https://godbolt.org/z/4TsPW41d5

Thanks, updated with the avr RUN lines.

[vitalybuka]

Breaks the bot https://lab.llvm.org/buildbot/#/builders/74/builds/21336/steps/13/logs/stdio

Please take a look or revert?

Note: msan_track_origins step may have useful details.

[tbaeder]

In D155568#4596497, @vitalybuka wrote:

Breaks the bot https://lab.llvm.org/buildbot/#/builders/74/builds/21336/steps/13/logs/stdio

Please take a look or revert?

Note: msan_track_origins step may have useful details.

Wow, sorry. That was too late in my day. I have a pretty good idea of why it's failing but I'll try to reproduce with an msan enabled build.

[tbaeder]

This landed as 673ef8ceaece6c9a7194474ef7d97b3b240c0dc5

clang/lib/AST/Interp/Integral.h

public:
   template <unsigned DstBits, bool DstSign>
   explicit operator Integral<DstBits, DstSign>() const {
     return Integral<DstBits, DstSign>(V);
   }
 
   explicit operator unsigned() const { return V; }
   explicit operator int64_t() const { return V; }
   explicit operator uint64_t() const { return V; }
+  explicit operator int32_t() const { return V; }
 
   APSInt toAPSInt() const {
     return APSInt(APInt(Bits, static_cast<uint64_t>(V), Signed), !Signed);
   }
   APSInt toAPSInt(unsigned NumBits) const {
     if constexpr (Signed)
       return APSInt(toAPSInt().sextOrTrunc(NumBits), !Signed);
     else

clang/lib/AST/Interp/InterpBuiltin.cpp

static APSInt peekToAPSInt(InterpStack &Stk, PrimType T) {
   INT_TYPE_SWITCH(T, {
     T Val = Stk.peek<T>();
     R = APSInt(APInt(T::bitWidth(), static_cast<uint64_t>(Val), T::isSigned()));
   });
 
   return R;
 }
 
+/// Pushes \p Val to the stack, as a target-dependent 'int'.
+static void pushInt(InterpState &S, int32_t Val) {
+  const TargetInfo &TI = S.getCtx().getTargetInfo();
+  unsigned IntWidth = TI.getIntWidth();
+
+  if (IntWidth == 32)
+    S.Stk.push<Integral<32, true>>(Integral<32, true>::from(Val));
+  else if (IntWidth == 16)
+    S.Stk.push<Integral<16, true>>(Integral<16, true>::from(Val));
+  else
+    llvm_unreachable("Int isn't 16 or 32 bit?");
+}
+
+static bool retInt(InterpState &S, CodePtr OpPC, APValue &Result) {
+  const TargetInfo &TI = S.getCtx().getTargetInfo();
+  unsigned IntWidth = TI.getIntWidth();
+
+  if (IntWidth == 32)
+    return Ret<PT_Sint32>(S, OpPC, Result);
+  else if (IntWidth == 16)
+    return Ret<PT_Sint16>(S, OpPC, Result);
+  llvm_unreachable("Int isn't 16 or 32 bit?");
+}
+
 static bool interp__builtin_strcmp(InterpState &S, CodePtr OpPC,
                                    const InterpFrame *Frame) {
   const Pointer &A = getParam<Pointer>(Frame, 0);
   const Pointer &B = getParam<Pointer>(Frame, 1);
 
   if (!CheckLive(S, OpPC, A, AK_Read) || !CheckLive(S, OpPC, B, AK_Read))
     return false;
 
for (;; ++IndexA, ++IndexB) {
     } else if (CA < CB) {
       Result = -1;
       break;
     }
     if (CA == 0 || CB == 0)
       break;
   }
 
-  S.Stk.push<Integral<32, true>>(Integral<32, true>::from(Result));
+  pushInt(S, Result);
   return true;
 }
 
 static bool interp__builtin_nan(InterpState &S, CodePtr OpPC,
                                 const InterpFrame *Frame, const Function *F,
                                 bool Signaling) {
   const Pointer &Arg = getParam<Pointer>(Frame, 0);
 

 
 /// Defined as __builtin_isnan(...), to accommodate the fact that it can
 /// take a float, double, long double, etc.
 /// But for us, that's all a Floating anyway.
 static bool interp__builtin_isnan(InterpState &S, CodePtr OpPC,
                                   const InterpFrame *Frame, const Function *F) {
   const Floating &Arg = S.Stk.peek<Floating>();
 
-  S.Stk.push<Integral<32, true>>(Integral<32, true>::from(Arg.isNan()));
+  pushInt(S, Arg.isNan());
   return true;
 }
 
 static bool interp__builtin_isinf(InterpState &S, CodePtr OpPC,
                                   const InterpFrame *Frame, const Function *F,
                                   bool CheckSign) {
   const Floating &Arg = S.Stk.peek<Floating>();
   bool IsInf = Arg.isInf();
 
   if (CheckSign)
-    S.Stk.push<Integral<32, true>>(
-        Integral<32, true>::from(IsInf ? (Arg.isNegative() ? -1 : 1) : 0));
+    pushInt(S, IsInf ? (Arg.isNegative() ? -1 : 1) : 0);
   else
-    S.Stk.push<Integral<32, true>>(Integral<32, true>::from(Arg.isInf()));
+    pushInt(S, Arg.isInf());
   return true;
 }
 
 static bool interp__builtin_isfinite(InterpState &S, CodePtr OpPC,
                                      const InterpFrame *Frame,
                                      const Function *F) {
   const Floating &Arg = S.Stk.peek<Floating>();
 
-  S.Stk.push<Integral<32, true>>(Integral<32, true>::from(Arg.isFinite()));
+  pushInt(S, Arg.isFinite());
   return true;
 }
 
 static bool interp__builtin_isnormal(InterpState &S, CodePtr OpPC,
                                      const InterpFrame *Frame,
                                      const Function *F) {
   const Floating &Arg = S.Stk.peek<Floating>();
 
-  S.Stk.push<Integral<32, true>>(Integral<32, true>::from(Arg.isNormal()));
+  pushInt(S, Arg.isNormal());
   return true;
 }
 
 /// First parameter to __builtin_isfpclass is the floating value, the
 /// second one is an integral value.
 static bool interp__builtin_isfpclass(InterpState &S, CodePtr OpPC,
                                       const InterpFrame *Frame,
                                       const Function *Func,
                                       const CallExpr *Call) {
   PrimType FPClassArgT = *S.getContext().classify(Call->getArg(1)->getType());
   APSInt FPClassArg = peekToAPSInt(S.Stk, FPClassArgT);
   const Floating &F =
       S.Stk.peek<Floating>(align(primSize(FPClassArgT) + primSize(PT_Float)));
 
   int32_t Result =
       static_cast<int32_t>((F.classify() & FPClassArg).getZExtValue());
-  S.Stk.push<Integral<32, true>>(Integral<32, true>::from(Result));
+  pushInt(S, Result);
 
   return true;
 }
 
-/// Five int32 values followed by one floating value.
+/// Five int values followed by one floating value.
 static bool interp__builtin_fpclassify(InterpState &S, CodePtr OpPC,
                                        const InterpFrame *Frame,
                                        const Function *Func) {
   const Floating &Val = S.Stk.peek<Floating>();
 
   unsigned Index;
   switch (Val.getCategory()) {
   case APFloat::fcNaN:
case APFloat::fcZero:
     Index = 4;
     break;
   }
 
   // The last argument is first on the stack.
   unsigned Offset = align(primSize(PT_Float)) +
                     ((1 + (4 - Index)) * align(primSize(PT_Sint32)));
 
+  // FIXME: The size of the value we're peeking here is target-dependent.
   const Integral<32, true> &I = S.Stk.peek<Integral<32, true>>(Offset);
-  S.Stk.push<Integral<32, true>>(I);
+  pushInt(S, static_cast<int32_t>(I));
   return true;
 }
 
 // The C standard says "fabs raises no floating-point exceptions,
 // even if x is a signaling NaN. The returned value is independent of
 // the current rounding direction mode."  Therefore constant folding can
 // proceed without regard to the floating point settings.
 // Reference, WG14 N2478 F.10.4.3
bool InterpretBuiltin(InterpState &S, CodePtr OpPC, const Function *F,
   switch (F->getBuiltinID()) {
   case Builtin::BI__builtin_is_constant_evaluated:
     S.Stk.push<Boolean>(Boolean::from(S.inConstantContext()));
     return Ret<PT_Bool>(S, OpPC, Dummy);
   case Builtin::BI__builtin_assume:
     return RetVoid(S, OpPC, Dummy);
   case Builtin::BI__builtin_strcmp:
     if (interp__builtin_strcmp(S, OpPC, Frame))
-      return Ret<PT_Sint32>(S, OpPC, Dummy);
+      return retInt(S, OpPC, Dummy);
     break;
   case Builtin::BI__builtin_nan:
   case Builtin::BI__builtin_nanf:
   case Builtin::BI__builtin_nanl:
   case Builtin::BI__builtin_nanf16:
   case Builtin::BI__builtin_nanf128:
     if (interp__builtin_nan(S, OpPC, Frame, F, /*Signaling=*/false))
       return Ret<PT_Float>(S, OpPC, Dummy);
bool InterpretBuiltin(InterpState &S, CodePtr OpPC, const Function *F,
   case Builtin::BI__builtin_fmaxf16:
   case Builtin::BI__builtin_fmaxf128:
     if (interp__builtin_fmax(S, OpPC, Frame, F))
       return Ret<PT_Float>(S, OpPC, Dummy);
     break;
 
   case Builtin::BI__builtin_isnan:
     if (interp__builtin_isnan(S, OpPC, Frame, F))
-      return Ret<PT_Sint32>(S, OpPC, Dummy);
+      return retInt(S, OpPC, Dummy);
     break;
 
   case Builtin::BI__builtin_isinf:
     if (interp__builtin_isinf(S, OpPC, Frame, F, /*Sign=*/false))
-      return Ret<PT_Sint32>(S, OpPC, Dummy);
+      return retInt(S, OpPC, Dummy);
     break;
 
   case Builtin::BI__builtin_isinf_sign:
     if (interp__builtin_isinf(S, OpPC, Frame, F, /*Sign=*/true))
-      return Ret<PT_Sint32>(S, OpPC, Dummy);
+      return retInt(S, OpPC, Dummy);
     break;
 
   case Builtin::BI__builtin_isfinite:
     if (interp__builtin_isfinite(S, OpPC, Frame, F))
-      return Ret<PT_Sint32>(S, OpPC, Dummy);
+      return retInt(S, OpPC, Dummy);
     break;
   case Builtin::BI__builtin_isnormal:
     if (interp__builtin_isnormal(S, OpPC, Frame, F))
-      return Ret<PT_Sint32>(S, OpPC, Dummy);
+      return retInt(S, OpPC, Dummy);
     break;
   case Builtin::BI__builtin_isfpclass:
     if (interp__builtin_isfpclass(S, OpPC, Frame, F, Call))
-      return Ret<PT_Sint32>(S, OpPC, Dummy);
+      return retInt(S, OpPC, Dummy);
     break;
   case Builtin::BI__builtin_fpclassify:
     if (interp__builtin_fpclassify(S, OpPC, Frame, F))
-      return Ret<PT_Sint32>(S, OpPC, Dummy);
+      return retInt(S, OpPC, Dummy);
     break;
 
   case Builtin::BI__builtin_fabs:
   case Builtin::BI__builtin_fabsf:
   case Builtin::BI__builtin_fabsl:
   case Builtin::BI__builtin_fabsf128:
     if (interp__builtin_fabs(S, OpPC, Frame, F))
       return Ret<PT_Float>(S, OpPC, Dummy);

clang/test/AST/Interp/builtin-functions.cpp

 // RUN: %clang_cc1 -fexperimental-new-constant-interpreter %s -verify
 // RUN: %clang_cc1 -verify=ref %s -Wno-constant-evaluated
 // RUN: %clang_cc1 -std=c++20 -fexperimental-new-constant-interpreter %s -verify
 // RUN: %clang_cc1 -std=c++20 -verify=ref %s -Wno-constant-evaluated
+// RUN: %clang_cc1 -triple avr -std=c++20 -fexperimental-new-constant-interpreter %s -verify
+// RUN: %clang_cc1 -triple avr -std=c++20 -verify=ref %s -Wno-constant-evaluated
 
 
 namespace strcmp {
   constexpr char kFoobar[6] = {'f','o','o','b','a','r'};
   constexpr char kFoobazfoobar[12] = {'f','o','o','b','a','z','f','o','o','b','a','r'};
 
   static_assert(__builtin_strcmp("", "") == 0, "");
   static_assert(__builtin_strcmp("abab", "abab") == 0, "");
namespace isfpclass {
   char isfpclass_inf_pos_0[__builtin_isfpclass(__builtin_inf(), 0x0200) ? 1 : -1]; // fcPosInf
   char isfpclass_inf_pos_1[!__builtin_isfpclass(__builtin_inff(), 0x0004) ? 1 : -1]; // fcNegInf
   char isfpclass_inf_pos_2[__builtin_isfpclass(__builtin_infl(), 0x0207) ? 1 : -1]; // fcSNan|fcQNan|fcNegInf|fcPosInf
   char isfpclass_inf_pos_3[!__builtin_isfpclass(__builtin_inf(), 0x01F8) ? 1 : -1]; // fcFinite
   char isfpclass_pos_0    [__builtin_isfpclass(1.0, 0x0100) ? 1 : -1]; // fcPosNormal
   char isfpclass_pos_1    [!__builtin_isfpclass(1.0f, 0x0008) ? 1 : -1]; // fcNegNormal
   char isfpclass_pos_2    [__builtin_isfpclass(1.0L, 0x01F8) ? 1 : -1]; // fcFinite
   char isfpclass_pos_3    [!__builtin_isfpclass(1.0, 0x0003) ? 1 : -1]; // fcSNan|fcQNan
+#ifndef __AVR__
   char isfpclass_pdenorm_0[__builtin_isfpclass(1.0e-40f, 0x0080) ? 1 : -1]; // fcPosSubnormal
   char isfpclass_pdenorm_1[__builtin_isfpclass(1.0e-310, 0x01F8) ? 1 : -1]; // fcFinite
   char isfpclass_pdenorm_2[!__builtin_isfpclass(1.0e-40f, 0x003C) ? 1 : -1]; // fcNegative
   char isfpclass_pdenorm_3[!__builtin_isfpclass(1.0e-310, 0x0207) ? 1 : -1]; // ~fcFinite
+#endif
   char isfpclass_pzero_0  [__builtin_isfpclass(0.0f, 0x0060) ? 1 : -1]; // fcZero
   char isfpclass_pzero_1  [__builtin_isfpclass(0.0, 0x01F8) ? 1 : -1]; // fcFinite
   char isfpclass_pzero_2  [!__builtin_isfpclass(0.0L, 0x0020) ? 1 : -1]; // fcNegZero
   char isfpclass_pzero_3  [!__builtin_isfpclass(0.0, 0x0003) ? 1 : -1]; // fcNan
   char isfpclass_nzero_0  [__builtin_isfpclass(-0.0f, 0x0060) ? 1 : -1]; // fcZero
   char isfpclass_nzero_1  [__builtin_isfpclass(-0.0, 0x01F8) ? 1 : -1]; // fcFinite
   char isfpclass_nzero_2  [!__builtin_isfpclass(-0.0L, 0x0040) ? 1 : -1]; // fcPosZero
   char isfpclass_nzero_3  [!__builtin_isfpclass(-0.0, 0x0003) ? 1 : -1]; // fcNan
   char isfpclass_ndenorm_0[__builtin_isfpclass(-1.0e-40f, 0x0010) ? 1 : -1]; // fcNegSubnormal
-  char isfpclass_ndenorm_1[__builtin_isfpclass(-1.0e-310, 0x01F8) ? 1 : -1]; // fcFinite
   char isfpclass_ndenorm_2[!__builtin_isfpclass(-1.0e-40f, 0x03C0) ? 1 : -1]; // fcPositive
+#ifndef __AVR__
+  char isfpclass_ndenorm_1[__builtin_isfpclass(-1.0e-310, 0x01F8) ? 1 : -1]; // fcFinite
   char isfpclass_ndenorm_3[!__builtin_isfpclass(-1.0e-310, 0x0207) ? 1 : -1]; // ~fcFinite
+#endif
   char isfpclass_neg_0    [__builtin_isfpclass(-1.0, 0x0008) ? 1 : -1]; // fcNegNormal
   char isfpclass_neg_1    [!__builtin_isfpclass(-1.0f, 0x00100) ? 1 : -1]; // fcPosNormal
   char isfpclass_neg_2    [__builtin_isfpclass(-1.0L, 0x01F8) ? 1 : -1]; // fcFinite
   char isfpclass_neg_3    [!__builtin_isfpclass(-1.0, 0x0003) ? 1 : -1]; // fcSNan|fcQNan
   char isfpclass_inf_neg_0[__builtin_isfpclass(-__builtin_inf(), 0x0004) ? 1 : -1]; // fcNegInf
   char isfpclass_inf_neg_1[!__builtin_isfpclass(-__builtin_inff(), 0x0200) ? 1 : -1]; // fcPosInf
   char isfpclass_inf_neg_2[__builtin_isfpclass(-__builtin_infl(), 0x0207) ? 1 : -1]; // ~fcFinite
   char isfpclass_inf_neg_3[!__builtin_isfpclass(-__builtin_inf(), 0x03C0) ? 1 : -1]; // fcPositive
   char isfpclass_qnan_0   [__builtin_isfpclass(__builtin_nan(""), 0x0002) ? 1 : -1]; // fcQNan
   char isfpclass_qnan_1   [!__builtin_isfpclass(__builtin_nanf(""), 0x0001) ? 1 : -1]; // fcSNan
   char isfpclass_qnan_2   [__builtin_isfpclass(__builtin_nanl(""), 0x0207) ? 1 : -1]; // ~fcFinite
   char isfpclass_qnan_3   [!__builtin_isfpclass(__builtin_nan(""), 0x01F8) ? 1 : -1]; // fcFinite
   char isfpclass_snan_0   [__builtin_isfpclass(__builtin_nansf(""), 0x0001) ? 1 : -1]; // fcSNan
   char isfpclass_snan_1   [!__builtin_isfpclass(__builtin_nans(""), 0x0002) ? 1 : -1]; // fcQNan
   char isfpclass_snan_2   [__builtin_isfpclass(__builtin_nansl(""), 0x0207) ? 1 : -1]; // ~fcFinite
   char isfpclass_snan_3   [!__builtin_isfpclass(__builtin_nans(""), 0x01F8) ? 1 : -1]; // fcFinite
 }
 
 namespace fpclassify {
   char classify_nan     [__builtin_fpclassify(+1, -1, -1, -1, -1, __builtin_nan(""))];
   char classify_snan    [__builtin_fpclassify(+1, -1, -1, -1, -1, __builtin_nans(""))];
   char classify_inf     [__builtin_fpclassify(-1, +1, -1, -1, -1, __builtin_inf())];
   char classify_neg_inf [__builtin_fpclassify(-1, +1, -1, -1, -1, -__builtin_inf())];
   char classify_normal  [__builtin_fpclassify(-1, -1, +1, -1, -1, 1.539)];
+#ifndef __AVR__
   char classify_normal2 [__builtin_fpclassify(-1, -1, +1, -1, -1, 1e-307)];
   char classify_denorm  [__builtin_fpclassify(-1, -1, -1, +1, -1, 1e-308)];
   char classify_denorm2 [__builtin_fpclassify(-1, -1, -1, +1, -1, -1e-308)];
+#endif
   char classify_zero    [__builtin_fpclassify(-1, -1, -1, -1, +1, 0.0)];
   char classify_neg_zero[__builtin_fpclassify(-1, -1, -1, -1, +1, -0.0)];
   char classify_subnorm [__builtin_fpclassify(-1, -1, -1, +1, -1, 1.0e-38f)];
 }
 
 namespace fabs {
   static_assert(__builtin_fabs(-14.0) == 14.0, "");
 }