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

[Timm Bäder via Phabricator via cfe-commits]

tbaeder closed this revision.
tbaeder added a comment.

This landed as 673ef8ceaece6c9a7194474ef7d97b3b240c0dc5 



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155568/new/

https://reviews.llvm.org/D155568

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added a comment.

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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155568/new/

https://reviews.llvm.org/D155568

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka added a comment.

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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155568/new/

https://reviews.llvm.org/D155568

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Timm Bäder via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG91af0d0a6698: [clang][Interp] Make sure we push integers of 
the correct size (authored by tbaeder).

Changed prior to commit:
  https://reviews.llvm.org/D155568?vs=545349=551043#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155568/new/

https://reviews.llvm.org/D155568

Files:
  clang/lib/AST/Interp/Integral.h
  clang/lib/AST/Interp/InterpBuiltin.cpp
  clang/test/AST/Interp/builtin-functions.cpp

Index: clang/test/AST/Interp/builtin-functions.cpp
===
--- clang/test/AST/Interp/builtin-functions.cpp
+++ clang/test/AST/Interp/builtin-functions.cpp
@@ -2,6 +2,8 @@
 // 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 {
@@ -96,10 +98,12 @@
   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
@@ -109,9 +113,11 @@
   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
@@ -136,9 +142,11 @@
   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)];
Index: clang/lib/AST/Interp/InterpBuiltin.cpp
===
--- clang/lib/AST/Interp/InterpBuiltin.cpp
+++ clang/lib/AST/Interp/InterpBuiltin.cpp
@@ -32,6 +32,30 @@
   return R;
 }
 
+/// Pushes \p Val to the stack, as a target-dependent 'int'.
+static void pushInt(InterpState , int32_t Val) {
+  const TargetInfo  = S.getCtx().getTargetInfo();
+  unsigned IntWidth = TI.getIntWidth();
+
+  if (IntWidth == 32)
+S.Stk.push>(Integral<32, true>::from(Val));
+  else if (IntWidth == 16)
+S.Stk.push>(Integral<16, true>::from(Val));
+  else
+llvm_unreachable("Int isn't 16 or 32 bit?");
+}
+
+static bool retInt(InterpState , CodePtr OpPC, APValue ) {
+  const TargetInfo  = S.getCtx().getTargetInfo();
+  unsigned IntWidth = TI.getIntWidth();
+
+  if (IntWidth == 32)
+return Ret(S, OpPC, Result);
+  else if (IntWidth == 16)
+return Ret(S, OpPC, Result);
+  llvm_unreachable("Int isn't 16 or 32 bit?");
+}
+
 static bool interp__builtin_strcmp(InterpState , CodePtr OpPC,
const InterpFrame *Frame) {
   const Pointer  

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

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added a comment.

In D155568#4542764 , @aaron.ballman 
wrote:

> In D155568#4525488 , @tbaeder wrote:
>
>> This patch is missing the `Ret` 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.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155568/new/

https://reviews.llvm.org/D155568

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Timm Bäder via Phabricator via cfe-commits]

tbaeder updated this revision to Diff 545349.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155568/new/

https://reviews.llvm.org/D155568

Files:
  clang/lib/AST/Interp/Integral.h
  clang/lib/AST/Interp/InterpBuiltin.cpp
  clang/test/AST/Interp/builtin-functions.cpp

Index: clang/test/AST/Interp/builtin-functions.cpp
===
--- clang/test/AST/Interp/builtin-functions.cpp
+++ clang/test/AST/Interp/builtin-functions.cpp
@@ -2,6 +2,8 @@
 // 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 {
@@ -93,10 +95,12 @@
   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
@@ -106,9 +110,11 @@
   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
@@ -133,9 +139,11 @@
   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)];
@@ -145,6 +153,7 @@
   static_assert(__builtin_fabs(-14.0) == 14.0, "");
 }
 
+#ifndef __AVR__
 namespace ArithmeticFence {
   constexpr double d = __arithmetic_fence(13.0);
   static_assert(d == 13.0, "");
@@ -157,3 +166,4 @@
   _Complex double CD = __arithmetic_fence(CD);
 #endif
 }
+#endif
Index: clang/lib/AST/Interp/InterpBuiltin.cpp
===
--- clang/lib/AST/Interp/InterpBuiltin.cpp
+++ clang/lib/AST/Interp/InterpBuiltin.cpp
@@ -32,6 +32,30 @@
   return R;
 }
 
+/// Pushes \p Val to the stack, as a target-dependent 'int'.
+static void pushInt(InterpState , int32_t Val) {
+  const TargetInfo  = S.getCtx().getTargetInfo();
+  unsigned IntWidth = TI.getIntWidth();
+
+  if (IntWidth == 32)
+S.Stk.push>(Integral<32, true>::from(Val));
+  else if (IntWidth == 16)
+S.Stk.push>(Integral<16, true>::from(Val));
+  else
+llvm_unreachable("Int isn't 16 or 32 bit?");
+}
+
+static bool retInt(InterpState , CodePtr OpPC, APValue ) {
+  const TargetInfo  = S.getCtx().getTargetInfo();
+  unsigned IntWidth = TI.getIntWidth();
+
+  if (IntWidth == 32)
+return Ret(S, OpPC, Result);
+  else if (IntWidth == 16)
+return Ret(S, OpPC, Result);
+  llvm_unreachable("Int isn't 16 or 32 bit?");
+}
+
 static bool interp__builtin_strcmp(InterpState , CodePtr OpPC,
const InterpFrame *Frame) {
   const Pointer  = getParam(Frame, 0);
@@ -67,7 

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D155568#4525488 , @tbaeder wrote:

> This patch is missing the `Ret` 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


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155568/new/

https://reviews.llvm.org/D155568

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Timm Bäder via Phabricator via cfe-commits]

tbaeder updated this revision to Diff 543321.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155568/new/

https://reviews.llvm.org/D155568

Files:
  clang/lib/AST/Interp/Integral.h
  clang/lib/AST/Interp/InterpBuiltin.cpp

Index: clang/lib/AST/Interp/InterpBuiltin.cpp
===
--- clang/lib/AST/Interp/InterpBuiltin.cpp
+++ clang/lib/AST/Interp/InterpBuiltin.cpp
@@ -32,6 +32,30 @@
   return R;
 }
 
+/// Pushes \p Val to the stack, as a target-dependent 'int'.
+static void pushInt(InterpState , int32_t Val) {
+  const TargetInfo  = S.getCtx().getTargetInfo();
+  unsigned IntWidth = TI.getIntWidth();
+
+  if (IntWidth == 32)
+S.Stk.push>(Integral<32, true>::from(Val));
+  else if (IntWidth == 16)
+S.Stk.push>(Integral<16, true>::from(Val));
+  else
+llvm_unreachable("Int isn't 16 or 32 bit?");
+}
+
+static bool retInt(InterpState , CodePtr OpPC, APValue ) {
+  const TargetInfo  = S.getCtx().getTargetInfo();
+  unsigned IntWidth = TI.getIntWidth();
+
+  if (IntWidth == 32)
+return Ret(S, OpPC, Result);
+  else if (IntWidth == 16)
+return Ret(S, OpPC, Result);
+  llvm_unreachable("Int isn't 16 or 32 bit?");
+}
+
 static bool interp__builtin_strcmp(InterpState , CodePtr OpPC,
const InterpFrame *Frame) {
   const Pointer  = getParam(Frame, 0);
@@ -67,7 +91,7 @@
   break;
   }
 
-  S.Stk.push>(Integral<32, true>::from(Result));
+  pushInt(S, Result);
   return true;
 }
 
@@ -197,7 +221,7 @@
   const InterpFrame *Frame, const Function *F) {
   const Floating  = S.Stk.peek();
 
-  S.Stk.push>(Integral<32, true>::from(Arg.isNan()));
+  pushInt(S, Arg.isNan());
   return true;
 }
 
@@ -208,10 +232,9 @@
   bool IsInf = Arg.isInf();
 
   if (CheckSign)
-S.Stk.push>(
-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>::from(Arg.isInf()));
+pushInt(S, Arg.isInf());
   return true;
 }
 
@@ -220,7 +243,7 @@
  const Function *F) {
   const Floating  = S.Stk.peek();
 
-  S.Stk.push>(Integral<32, true>::from(Arg.isFinite()));
+  pushInt(S, Arg.isFinite());
   return true;
 }
 
@@ -229,7 +252,7 @@
  const Function *F) {
   const Floating  = S.Stk.peek();
 
-  S.Stk.push>(Integral<32, true>::from(Arg.isNormal()));
+  pushInt(S, Arg.isNormal());
   return true;
 }
 
@@ -246,12 +269,12 @@
 
   int32_t Result =
   static_cast((F.classify() & FPClassArg).getZExtValue());
-  S.Stk.push>(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 , CodePtr OpPC,
const InterpFrame *Frame,
const Function *Func) {
@@ -277,8 +300,9 @@
   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>  = S.Stk.peek>(Offset);
-  S.Stk.push>(I);
+  pushInt(S, static_cast(I));
   return true;
 }
 
@@ -327,7 +351,7 @@
 return RetVoid(S, OpPC, Dummy);
   case Builtin::BI__builtin_strcmp:
 if (interp__builtin_strcmp(S, OpPC, Frame))
-  return Ret(S, OpPC, Dummy);
+  return retInt(S, OpPC, Dummy);
 break;
   case Builtin::BI__builtin_nan:
   case Builtin::BI__builtin_nanf:
@@ -387,34 +411,34 @@
 
   case Builtin::BI__builtin_isnan:
 if (interp__builtin_isnan(S, OpPC, Frame, F))
-  return Ret(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(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(S, OpPC, Dummy);
+  return retInt(S, OpPC, Dummy);
 break;
 
   case Builtin::BI__builtin_isfinite:
 if (interp__builtin_isfinite(S, OpPC, Frame, F))
-  return Ret(S, OpPC, Dummy);
+  return retInt(S, OpPC, Dummy);
 break;
   case Builtin::BI__builtin_isnormal:
 if (interp__builtin_isnormal(S, OpPC, Frame, F))
-  return Ret(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(S, OpPC, Dummy);
+  return retInt(S, OpPC, Dummy);
 break;
   case Builtin::BI__builtin_fpclassify:
 if (interp__builtin_fpclassify(S, OpPC, Frame, F))
-  return Ret(S, OpPC, Dummy);
+  return retInt(S, OpPC, Dummy);
 

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

[Timm Bäder via Phabricator via cfe-commits]

tbaeder updated this revision to Diff 543320.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155568/new/

https://reviews.llvm.org/D155568

Files:
  clang/lib/AST/Interp/Integral.h
  clang/lib/AST/Interp/InterpBuiltin.cpp

Index: clang/lib/AST/Interp/InterpBuiltin.cpp
===
--- clang/lib/AST/Interp/InterpBuiltin.cpp
+++ clang/lib/AST/Interp/InterpBuiltin.cpp
@@ -32,6 +32,30 @@
   return R;
 }
 
+/// Pushes \p Val to the stack, as a target-dependent 'int'.
+static void pushInt(InterpState , int32_t Val) {
+  const TargetInfo  = S.getCtx().getTargetInfo();
+  unsigned IntWidth = TI.getIntWidth();
+
+  if (IntWidth == 32)
+S.Stk.push>(Integral<32, true>::from(Val));
+  else if (IntWidth == 16)
+S.Stk.push>(Integral<16, true>::from(Val));
+  else
+llvm_unreachable("Int isn't 16 or 32 bit?");
+}
+
+static bool retInt(InterpState , CodePtr OpPC, APValue ) {
+  const TargetInfo  = S.getCtx().getTargetInfo();
+  unsigned IntWidth = TI.getIntWidth();
+
+  if (IntWidth == 32)
+return Ret(S, OpPC, Result);
+  else if (IntWidth == 16)
+return Ret(S, OpPC, Result);
+  llvm_unreachable("Int isn't 16 or 32 bit?");
+}
+
 static bool interp__builtin_strcmp(InterpState , CodePtr OpPC,
const InterpFrame *Frame) {
   const Pointer  = getParam(Frame, 0);
@@ -67,7 +91,7 @@
   break;
   }
 
-  S.Stk.push>(Integral<32, true>::from(Result));
+  pushInt(S, Result);
   return true;
 }
 
@@ -197,7 +221,7 @@
   const InterpFrame *Frame, const Function *F) {
   const Floating  = S.Stk.peek();
 
-  S.Stk.push>(Integral<32, true>::from(Arg.isNan()));
+  pushInt(S, Arg.isNan());
   return true;
 }
 
@@ -208,10 +232,9 @@
   bool IsInf = Arg.isInf();
 
   if (CheckSign)
-S.Stk.push>(
-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>::from(Arg.isInf()));
+pushInt(S, Arg.isInf());
   return true;
 }
 
@@ -220,7 +243,7 @@
  const Function *F) {
   const Floating  = S.Stk.peek();
 
-  S.Stk.push>(Integral<32, true>::from(Arg.isFinite()));
+  pushInt(S, Arg.isFinite());
   return true;
 }
 
@@ -229,7 +252,7 @@
  const Function *F) {
   const Floating  = S.Stk.peek();
 
-  S.Stk.push>(Integral<32, true>::from(Arg.isNormal()));
+  pushInt(S, Arg.isNormal());
   return true;
 }
 
@@ -246,12 +269,12 @@
 
   int32_t Result =
   static_cast((F.classify() & FPClassArg).getZExtValue());
-  S.Stk.push>(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 , CodePtr OpPC,
const InterpFrame *Frame,
const Function *Func) {
@@ -277,8 +300,9 @@
   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>  = S.Stk.peek>(Offset);
-  S.Stk.push>(I);
+  pushInt(S, static_cast(I));
   return true;
 }
 
@@ -327,7 +351,7 @@
 return RetVoid(S, OpPC, Dummy);
   case Builtin::BI__builtin_strcmp:
 if (interp__builtin_strcmp(S, OpPC, Frame))
-  return Ret(S, OpPC, Dummy);
+  return retInt(S, OpPC, Dummy);
 break;
   case Builtin::BI__builtin_nan:
   case Builtin::BI__builtin_nanf:
@@ -387,34 +411,34 @@
 
   case Builtin::BI__builtin_isnan:
 if (interp__builtin_isnan(S, OpPC, Frame, F))
-  return Ret(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(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(S, OpPC, Dummy);
+  return retInt(S, OpPC, Dummy);
 break;
 
   case Builtin::BI__builtin_isfinite:
 if (interp__builtin_isfinite(S, OpPC, Frame, F))
-  return Ret(S, OpPC, Dummy);
+  return retInt(S, OpPC, Dummy);
 break;
   case Builtin::BI__builtin_isnormal:
 if (interp__builtin_isnormal(S, OpPC, Frame, F))
-  return Ret(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(S, OpPC, Dummy);
+  return retInt(S, OpPC, Dummy);
 break;
   case Builtin::BI__builtin_fpclassify:
 if (interp__builtin_fpclassify(S, OpPC, Frame, F))
-  return Ret(S, OpPC, Dummy);
+  return retInt(S, OpPC, Dummy);
 

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

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added a comment.

This patch is missing the `Ret` 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?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155568/new/

https://reviews.llvm.org/D155568

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added inline comments.



Comment at: clang/lib/AST/Interp/InterpBuiltin.cpp:36
+/// Pushes \p Val to the stack, as a target-dependent 'int'.
+static void pushInt(InterpState , int32_t Val) {
+  const TargetInfo  = S.getCtx().getTargetInfo();

aaron.ballman wrote:
> Do we need the same for unsigned int as well? (I'm thinking in terms of 
> pushing literal values like `0U`)
Theoretically yes, but all the ints we're pushing right now are signed AFAICS.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155568/new/

https://reviews.llvm.org/D155568

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

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




Comment at: clang/lib/AST/Interp/InterpBuiltin.cpp:36
+/// Pushes \p Val to the stack, as a target-dependent 'int'.
+static void pushInt(InterpState , int32_t Val) {
+  const TargetInfo  = S.getCtx().getTargetInfo();

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


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155568/new/

https://reviews.llvm.org/D155568

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Timm Bäder via Phabricator via cfe-commits]

tbaeder created this revision.
tbaeder added reviewers: aaron.ballman, erichkeane, shafik, cor3ntin.
Herald added a project: All.
tbaeder requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D155568

Files:
  clang/lib/AST/Interp/Integral.h
  clang/lib/AST/Interp/InterpBuiltin.cpp

Index: clang/lib/AST/Interp/InterpBuiltin.cpp
===
--- clang/lib/AST/Interp/InterpBuiltin.cpp
+++ clang/lib/AST/Interp/InterpBuiltin.cpp
@@ -32,6 +32,19 @@
   return R;
 }
 
+/// Pushes \p Val to the stack, as a target-dependent 'int'.
+static void pushInt(InterpState , int32_t Val) {
+  const TargetInfo  = S.getCtx().getTargetInfo();
+  unsigned IntWidth = TI.getIntWidth();
+
+  if (IntWidth == 32)
+S.Stk.push>(Integral<32, true>::from(Val));
+  else if (IntWidth == 16)
+S.Stk.push>(Integral<16, true>::from(Val));
+  else
+llvm_unreachable("Int isn't 16 or 32 bit?");
+}
+
 static bool interp__builtin_strcmp(InterpState , CodePtr OpPC,
const InterpFrame *Frame) {
   const Pointer  = getParam(Frame, 0);
@@ -67,7 +80,7 @@
   break;
   }
 
-  S.Stk.push>(Integral<32, true>::from(Result));
+  pushInt(S, Result);
   return true;
 }
 
@@ -201,7 +214,7 @@
   const InterpFrame *Frame, const Function *F) {
   const Floating  = S.Stk.peek();
 
-  S.Stk.push>(Integral<32, true>::from(Arg.isNan()));
+  pushInt(S, Arg.isNan());
   return true;
 }
 
@@ -212,10 +225,9 @@
   bool IsInf = Arg.isInf();
 
   if (CheckSign)
-S.Stk.push>(
-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>::from(Arg.isInf()));
+pushInt(S, Arg.isInf());
   return true;
 }
 
@@ -224,7 +236,7 @@
  const Function *F) {
   const Floating  = S.Stk.peek();
 
-  S.Stk.push>(Integral<32, true>::from(Arg.isFinite()));
+  pushInt(S, Arg.isFinite());
   return true;
 }
 
@@ -233,7 +245,7 @@
  const Function *F) {
   const Floating  = S.Stk.peek();
 
-  S.Stk.push>(Integral<32, true>::from(Arg.isNormal()));
+  pushInt(S, Arg.isNormal());
   return true;
 }
 
@@ -250,12 +262,12 @@
 
   int32_t Result =
   static_cast((F.classify() & FPClassArg).getZExtValue());
-  S.Stk.push>(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 , CodePtr OpPC,
const InterpFrame *Frame,
const Function *Func) {
@@ -281,8 +293,9 @@
   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>  = S.Stk.peek>(Offset);
-  S.Stk.push>(I);
+  pushInt(S, static_cast(I));
   return true;
 }
 
Index: clang/lib/AST/Interp/Integral.h
===
--- clang/lib/AST/Interp/Integral.h
+++ clang/lib/AST/Interp/Integral.h
@@ -95,6 +95,7 @@
   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(V), Signed), !Signed);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits