[clang][Interp] IntegralAP zero-init

[tbaederr]

Not adding any tests since I'm waiting for #68069

[llvmbot]

@llvm/pr-subscribers-clang

Changes

Not adding any tests since I'm waiting for #68069

---

Full diff: https://github.com/llvm/llvm-project/pull/68081.diff

4 Files Affected:

• (modified) clang/lib/AST/Interp/ByteCodeExprGen.cpp (+2-2)
• (modified) clang/lib/AST/Interp/IntegralAP.h (+5-4)
• (modified) clang/lib/AST/Interp/Interp.h (+10)
• (modified) clang/lib/AST/Interp/Opcodes.td (+14-1)

diff --git a/clang/lib/AST/Interp/ByteCodeExprGen.cpp b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
index e266804a4e75dea..9a2bbe5c1841208 100644
--- a/clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -1642,9 +1642,9 @@ bool ByteCodeExprGen<Emitter>::visitZeroInitializer(QualType QT,
   case PT_Uint64:
     return this->emitZeroUint64(E);
   case PT_IntAP:
+    return this->emitZeroIntAP(128, E); // FIXME: Ctx.getBitWidth()
   case PT_IntAPS:
-    assert(false);
-    return false;
+    return this->emitZeroIntAPS(128, E); // FIXME: Ctx.getBitWidth()
   case PT_Ptr:
     return this->emitNullPtr(E);
   case PT_FnPtr:
diff --git a/clang/lib/AST/Interp/IntegralAP.h b/clang/lib/AST/Interp/IntegralAP.h
index a8df431bef11784..bca39884ac1de88 100644
--- a/clang/lib/AST/Interp/IntegralAP.h
+++ b/clang/lib/AST/Interp/IntegralAP.h
@@ -100,12 +100,13 @@ template <bool Signed> class IntegralAP final {
   }
   static IntegralAP from(const Boolean &B) {
     assert(false);
-    return IntegralAP::zero();
+    return IntegralAP::zero(1);
   }
 
-  static IntegralAP zero() {
-    assert(false);
-    return IntegralAP(0);
+  static IntegralAP zero(int32_t BitWidth) {
+    APSInt V =
+        APSInt(APInt(BitWidth, static_cast<int64_t>(0), Signed), !Signed);
+    return IntegralAP(V);
   }
 
   // FIXME: This can't be static if the bitwidth depends on V.
diff --git a/clang/lib/AST/Interp/Interp.h b/clang/lib/AST/Interp/Interp.h
index 9d5ec3315415cf7..f768deca62b8b0a 100644
--- a/clang/lib/AST/Interp/Interp.h
+++ b/clang/lib/AST/Interp/Interp.h
@@ -1622,6 +1622,16 @@ bool Zero(InterpState &S, CodePtr OpPC) {
   return true;
 }
 
+static inline bool ZeroIntAP(InterpState &S, CodePtr OpPC, uint32_t BitWidth) {
+  S.Stk.push<IntegralAP<false>>(IntegralAP<false>::zero(BitWidth));
+  return true;
+}
+
+static inline bool ZeroIntAPS(InterpState &S, CodePtr OpPC, uint32_t BitWidth) {
+  S.Stk.push<IntegralAP<true>>(IntegralAP<true>::zero(BitWidth));
+  return true;
+}
+
 template <PrimType Name, class T = typename PrimConv<Name>::T>
 inline bool Null(InterpState &S, CodePtr OpPC) {
   S.Stk.push<T>();
diff --git a/clang/lib/AST/Interp/Opcodes.td b/clang/lib/AST/Interp/Opcodes.td
index 9fc4938bb37bde8..d78431c56645629 100644
--- a/clang/lib/AST/Interp/Opcodes.td
+++ b/clang/lib/AST/Interp/Opcodes.td
@@ -72,6 +72,11 @@ def IntegerTypeClass : TypeClass {
                Uint32, Sint64, Uint64, IntAP, IntAPS];
 }
 
+def FixedSizeIntegralTypeClass : TypeClass {
+  let Types = [Sint8, Uint8, Sint16, Uint16, Sint32,
+               Uint32, Sint64, Uint64, Bool];
+}
+
 def NumberTypeClass : TypeClass {
   let Types = !listconcat(IntegerTypeClass.Types, [Float]);
 }
@@ -243,10 +248,18 @@ def ConstBool : ConstOpcode<Bool, ArgBool>;
 
 // [] -> [Integer]
 def Zero : Opcode {
-  let Types = [AluTypeClass];
+  let Types = [FixedSizeIntegralTypeClass];
   let HasGroup = 1;
 }
 
+def ZeroIntAP : Opcode {
+  let Args = [ArgUint32];
+}
+
+def ZeroIntAPS : Opcode {
+  let Args = [ArgUint32];
+}
+
 // [] -> [Pointer]
 def Null : Opcode {
   let Types = [PtrTypeClass];

[tbaederr]

Ping

[AaronBallman]

Suggested change

	APSInt(APInt(BitWidth, static_cast<int64_t>(0), Signed), !Signed);
	APSInt(APInt(BitWidth, 0LL, Signed), !Signed);

we don't even need the LL do we?

[tbaederr]

I keep having to add this cast to int64_t because some Windows builders will otherwise complain because the APInt class has two constructors it can take:

APInt (unsigned numBits, uint64_t val, bool isSigned=false)
APInt (unsigned numBits, unsigned numWords, const uint64_t bigVal[])

and MSVC doesn't know which one to take.

[AaronBallman]

Oh wow, that's... neat. The explicit cast is also fine, but perhaps should be to uint64_t to remove one more implicit conversion step.

[AaronBallman]

Why does this not look at the value of B?

[tbaederr]

This is unimplemented right now, I just added the bitwidth to make it compile.

[AaronBallman]

Ah, please add FIXME comments to unimplemented stuff when you can.

(I'll be honest -- reviewing new interpreter patches is getting really difficult because of the interdependence between so many unrelated reviews and how much stuff is half-done. That's why these reviews keep getting delayed on my end, there's just too much to try to keep track of spread out across too many reviews. I wish we could find a way to improve this without impeding your ability to make progress. CC @shafik @erichkeane @cor3ntin as other folks doing a fair number of these reviews in case this is a shared concern and folks have ideas on how to improve it.)

[tbaederr]

I don't have such comments because I opted for assert(false) where things are unimplemented :)

I keep opening smaller PRs to make review easier because I thought that... makes it easier. If larger (but complete) patches are better for you, I can try to do that instead.

[tbaederr]

Removed that code in 66c9915.

[AaronBallman]

Aside from nits, this LGTM.

[AaronBallman]

LGTM!