[clang][Interp] Implement inv and neg unary operations

[tbaeder]

Summary

Implement inverting and negating values

[tbaeder]

clang/lib/AST/Interp/ByteCodeExprGen.cpp

134–148

Implementing the IntegralToBoolean cast is not necessary for this change, but makes more tests possible.

[shafik]

clang/lib/AST/Interp/ByteCodeExprGen.cpp

142

Can you explain this line?

clang/lib/AST/Interp/Interp.h

183

Shouldn't this be

[aaron.ballman]

I'd appreciate if we handled unary ~ and + soon after this

clang/lib/AST/Interp/ByteCodeExprGen.cpp

632

This is reachable and will always return true thanks to the magic of integer promotions:

void func() {
  bool b = true;
  b = ~b;
}

clang/test/AST/Interp/literals.cpp

2–15

[shafik]

clang/lib/AST/Interp/ByteCodeExprGen.cpp

142

Nevermind, I get it, you are emitting a 0 and then doing a not equal to zero to determine the result.

I feel like a comment explaining what is going on would be helpful for anyone not familiar with the code.

[tbaeder]

clang/lib/AST/Interp/ByteCodeExprGen.cpp

632

This code path is not just for boolean values though.

clang/lib/AST/Interp/Interp.h

183

Haha, yes. I already found this bug once before locally but now imported the broken version in my main branch. Thanks for catching that.

clang/test/AST/Interp/literals.cpp

2–15

That works as well but is also fixed after https://reviews.llvm.org/D132136

[tbaeder]

When changing the test to use -verify=test, I noticed that static_assert(-false) did not assert.

I had to implement integral casts (only from sint32 to bool for now) in this patch as well to make it work.

[tbaeder]

clang/lib/AST/Interp/Integral.h

175

I'm a bit out of my element with the template magic here.

[aaron.ballman]

clang/lib/AST/Interp/ByteCodeExprGen.cpp

129–130

632

All the more reason to be worried about ~ being marked as unreachable (I mostly worry because people often try to make bit fiddling operations constant expressions when possible, so this seems pretty likely to be hit). Perhaps make it into an assert(false && "operation not supported yet"); or something more loud?

clang/lib/AST/Interp/Integral.h

175

It mostly looks correct to me, but the part I'm struggling with is static_cast<Integral::T>; can you explain what you're trying to do there?

[tbaeder]

clang/lib/AST/Interp/ByteCodeExprGen.cpp

632

How is that different? It's just a marker for me so I know later in development that this is not implemented yet. And now that I'm writing this, I guess I see what you mean. Yes, it's not unreachable, but it shouldn't be reached when testing what is currently implemented.

clang/lib/AST/Interp/Integral.h

175

I was trying to implement creating an Integral from a Boolean; the static_cast casts to Integral::T (which in my case is int). It might not be needed.

[erichkeane]

clang/lib/AST/Interp/ByteCodeExprGen.cpp

632

Aaron: we tend to NEVER use 'assert(false...' and instead use llvm_unreachable for 'not implemented yet' quite often in clang. I don't see the semantic difference here?

[erichkeane]

clang/lib/AST/Interp/ByteCodeExprGen.cpp

632

Aaron clarified offline: the problem is that llvm_unreachable inserts optimization hints in non-asserts builds, so the result is optimizer-caused-UB here. The preference for 'assert(false', though it causing an odd state, at least doesn't allow the optimizer to go hog wild.

So I agree with Aaron here.

We DO do this wrong in some places, which is unfortunate, but should be considered mistakes, not precedent.

[aaron.ballman]

clang/lib/AST/Interp/Integral.h

175

Oh!!! I see now why there's so much confusion. You have template <typename T> and I was wondering why you were trying to use that here. I see now that there's Integral::T as a private member.

Can we please rename Integral::T to be something mildly descriptive? (I'm fine if that's done in a follow-up as an NFC change.)

[erichkeane]

clang/lib/AST/Interp/Integral.h

175

As Aaron said separately, the 'T' name of the integral member is bonkers and confusing!

So it appears the intent here is to take a non-integral 'Value', cast it to the underlying representation, and then call the version of this on 159.

From a 'style' perspective, I'd prefer this live right next to that version (since it is the inverse SFINAE). In fact, I'd probably prefer you switch this to an 'if constexpr' instead:

template <typename ValTy>
static Integral from (ValTy Value) {
    if constexpr (std::is_integral_v<T>) {
       return Integral(Value);
    } else {
       return Integral(static_cast<Integral::RepTy>(Value));
    }
 }

}

[tbaeder]

clang/lib/AST/Interp/Integral.h

175

That's a cool solution, thanks! And yes I'll push a renaming patch.

[aaron.ballman]

LGTM!

clang/lib/AST/Interp/Boolean.h

public:
   bool operator>(unsigned RHS) const { return static_cast<unsigned>(V) > RHS; }
 
   Boolean operator-() const { return Boolean(V); }
   Boolean operator~() const { return Boolean(true); }
 
   explicit operator unsigned() const { return V; }
   explicit operator int64_t() const { return V; }
   explicit operator uint64_t() const { return V; }
+  explicit operator int() const { return V; }
 
   APSInt toAPSInt() const {
     return APSInt(APInt(1, static_cast<uint64_t>(V), false), true);
   }
   APSInt toAPSInt(unsigned NumBits) const {
     return APSInt(toAPSInt().zextOrTrunc(NumBits), true);
   }
   APValue toAPValue() const { return APValue(toAPSInt()); }
static bool sub(Boolean A, Boolean B, unsigned OpBits, Boolean *R) {
     *R = Boolean(A.V ^ B.V);
     return false;
   }
 
   static bool mul(Boolean A, Boolean B, unsigned OpBits, Boolean *R) {
     *R = Boolean(A.V && B.V);
     return false;
   }
+
+  static bool inv(Boolean A, Boolean *R) {
+    *R = Boolean(!A.V);
+    return false;
+  }
+
+  static bool neg(Boolean A, Boolean *R) {
+    *R = Boolean(A.V);
+    return false;
+  }
 };
 
 inline llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const Boolean &B) {
   B.print(OS);
   return OS;
 }
 
 }  // namespace interp
 }  // namespace clang
 
 #endif

clang/lib/AST/Interp/ByteCodeExprGen.h

public:
 
   // Expression visitors - result returned on stack.
   bool VisitCastExpr(const CastExpr *E);
   bool VisitIntegerLiteral(const IntegerLiteral *E);
   bool VisitParenExpr(const ParenExpr *E);
   bool VisitBinaryOperator(const BinaryOperator *E);
   bool VisitCXXBoolLiteralExpr(const CXXBoolLiteralExpr *E);
   bool VisitCXXNullPtrLiteralExpr(const CXXNullPtrLiteralExpr *E);
+  bool VisitUnaryOperator(const UnaryOperator *E);
 
 protected:
   bool visitExpr(const Expr *E) override;
   bool visitDecl(const VarDecl *VD) override;
 
 protected:
   /// Emits scope cleanup instructions.
   void emitCleanup();

clang/lib/AST/Interp/ByteCodeExprGen.cpp

bool ByteCodeExprGen<Emitter>::VisitCastExpr(const CastExpr *CE) {
   case CK_ConstructorConversion:
   case CK_FunctionToPointerDecay:
   case CK_NonAtomicToAtomic:
   case CK_NoOp:
   case CK_UserDefinedConversion:
   case CK_NullToPointer:
     return this->Visit(SubExpr);
 
+  case CK_IntegralCast: {
+    Optional<PrimType> FromT = classify(SubExpr->getType());
+    Optional<PrimType> ToT = classify(CE->getType());
+    if (!FromT || !ToT)
+      return false;
+
+    if (!this->Visit(SubExpr))
+      return false;
+
+    return this->emitCast(*FromT, *ToT, CE);
+  }
+
   case CK_ToVoid:
     return discard(SubExpr);
 
-  default: {
-    // TODO: implement other casts.
-    return this->bail(CE);
+  case CK_IntegralToBoolean:
+    // Compare integral from Subexpr with 0
+    if (Optional<PrimType> T = classify(SubExpr->getType())) {
+      if (!this->Visit(SubExpr))
+        return false;
+
+      if (!this->emitConst(SubExpr, 0))
+        return false;
+
+      return this->emitNE(*T, SubExpr);
     }
+    return false;
+
+  default:
+    assert(false && "Cast not implemented");
   }
 }
 
 template <class Emitter>
 bool ByteCodeExprGen<Emitter>::VisitIntegerLiteral(const IntegerLiteral *LE) {
   if (DiscardResult)
     return true;
 
bool ByteCodeExprGen<Emitter>::VisitCXXNullPtrLiteralExpr(
     const CXXNullPtrLiteralExpr *E) {
   if (DiscardResult)
     return true;
 
   return this->emitNullPtr(E);
 }
 
 template <class Emitter>
+bool ByteCodeExprGen<Emitter>::VisitUnaryOperator(const UnaryOperator *E) {
+  if (!this->Visit(E->getSubExpr()))
+    return false;
+
+  switch (E->getOpcode()) {
+  case UO_PostInc: // x++
+  case UO_PostDec: // x--
+  case UO_PreInc:  // --x
+  case UO_PreDec:  // ++x
+    return false;
+
+  case UO_LNot: // !x
+    return this->emitInvBool(E);
+  case UO_Minus: // -x
+    if (Optional<PrimType> T = classify(E->getType()))
+      return this->emitNeg(*T, E);
+    return false;
+  case UO_Plus:  // +x
+    return true; // noop
+
+  case UO_AddrOf: // &x
+  case UO_Deref:  // *x
+  case UO_Not:    // ~x
+  case UO_Real:   // __real x
+  case UO_Imag:   // __imag x
+  case UO_Extension:
+  case UO_Coawait:
+    assert(false && "Unhandled opcode");
+  }
+
+  return false;
+}
+
+template <class Emitter>
 void ByteCodeExprGen<Emitter>::emitCleanup() {
   for (VariableScope<Emitter> *C = VarScope; C; C = C->getParent())
     C->emitDestruction();
 }
 
 namespace clang {
 namespace interp {
 
 template class ByteCodeExprGen<ByteCodeEmitter>;
 template class ByteCodeExprGen<EvalEmitter>;
 
 } // namespace interp
 } // namespace clang

clang/lib/AST/Interp/Integral.h

public:
 
   static Integral min(unsigned NumBits) {
     return Integral(Min);
   }
   static Integral max(unsigned NumBits) {
     return Integral(Max);
   }
 
-  template <typename T>
-  static std::enable_if_t<std::is_integral<T>::value, Integral> from(T Value) {
+  template <typename ValT> static Integral from(ValT Value) {
+    if constexpr (std::is_integral<T>::value)
       return Integral(Value);
+    else
+      return Integral::from(static_cast<Integral::T>(Value));
   }
 
   template <unsigned SrcBits, bool SrcSign>
   static std::enable_if_t<SrcBits != 0, Integral>
   from(Integral<SrcBits, SrcSign> Value) {
     return Integral(Value.V);
   }
 
   template <bool SrcSign> static Integral from(Integral<0, SrcSign> Value) {
     if (SrcSign)
       return Integral(Value.V.getSExtValue());
     else
       return Integral(Value.V.getZExtValue());
   }
 
   static Integral zero() { return from(0); }
 
   template <typename T> static Integral from(T Value, unsigned NumBits) {
     return Integral(Value);
   }
 
public:
   static bool sub(Integral A, Integral B, unsigned OpBits, Integral *R) {
     return CheckSubUB(A.V, B.V, R->V);
   }
 
   static bool mul(Integral A, Integral B, unsigned OpBits, Integral *R) {
     return CheckMulUB(A.V, B.V, R->V);
   }
 
+  static bool neg(Integral A, Integral *R) {
+    *R = -A;
+    return false;
+  }
+
 private:
   template <typename T>
   static std::enable_if_t<std::is_signed<T>::value, bool> CheckAddUB(T A, T B,
                                                                      T &R) {
     return llvm::AddOverflow<T>(A, B, R);
   }
 
   template <typename T>

clang/lib/AST/Interp/Interp.h

 bool Mul(InterpState &S, CodePtr OpPC) {
   const T &RHS = S.Stk.pop<T>();
   const T &LHS = S.Stk.pop<T>();
   const unsigned Bits = RHS.bitWidth() * 2;
   return AddSubMulHelper<T, T::mul, std::multiplies>(S, OpPC, Bits, LHS, RHS);
 }
 
 //===----------------------------------------------------------------------===//
+// Inv
+//===----------------------------------------------------------------------===//
+
+template <PrimType Name, class T = typename PrimConv<Name>::T>
+bool Inv(InterpState &S, CodePtr OpPC) {
+  using BoolT = PrimConv<PT_Bool>::T;
+  const T &Val = S.Stk.pop<T>();
+  const unsigned Bits = Val.bitWidth();
+  Boolean R;
+  Boolean::inv(BoolT::from(Val, Bits), &R);
+
+  S.Stk.push<BoolT>(R);
+  return true;
+}
+
+//===----------------------------------------------------------------------===//
+// Neg
+//===----------------------------------------------------------------------===//
+
+template <PrimType Name, class T = typename PrimConv<Name>::T>
+bool Neg(InterpState &S, CodePtr OpPC) {
+  const T &Val = S.Stk.pop<T>();
+  T Result;
+  T::neg(Val, &Result);
+
+  S.Stk.push<T>(Result);
+  return true;
+}
+
+//===----------------------------------------------------------------------===//
 // EQ, NE, GT, GE, LT, LE
 //===----------------------------------------------------------------------===//
 
 using CompareFn = llvm::function_ref<bool(ComparisonCategoryResult)>;
 
 template <typename T>
 bool CmpHelper(InterpState &S, CodePtr OpPC, CompareFn Fn) {
   using BoolT = PrimConv<PT_Bool>::T;

clang/lib/AST/Interp/Opcodes.td

def AluTypeClass : TypeClass {
   let Types = [Sint8, Uint8, Sint16, Uint16, Sint32,
                Uint32, Sint64, Uint64, Bool];
 }
 
 def PtrTypeClass : TypeClass {
   let Types = [Ptr];
 }
 
+def BoolTypeClass : TypeClass {
+  let Types = [Bool];
+}
+
 def AllTypeClass : TypeClass {
   let Types = !listconcat(AluTypeClass.Types, PtrTypeClass.Types);
 }
 
 def ComparableTypeClass : TypeClass {
   let Types = !listconcat(AluTypeClass.Types, [Ptr]);
 }
 

 // Binary operators.
 //===----------------------------------------------------------------------===//
 
 // [Real, Real] -> [Real]
 def Sub : AluOpcode;
 def Add : AluOpcode;
 def Mul : AluOpcode;
 
+
+//===----------------------------------------------------------------------===//
+// Unary operators.
+//===----------------------------------------------------------------------===//
+
+// [Real] -> [Real]
+def Inv: Opcode {
+  let Types = [BoolTypeClass];
+  let HasGroup = 1;
+}
+
+// [Real] -> [Real]
+def Neg: Opcode {
+  let Types = [AluTypeClass];
+  let HasGroup = 1;
+}
+
+//===----------------------------------------------------------------------===//
+// Cast.
+//===----------------------------------------------------------------------===//
+// TODO: Expand this to handle casts between more types.
+
+def Sint32TypeClass : TypeClass {
+  let Types = [Sint32];
+}
+
+def Cast: Opcode {
+  let Types = [BoolTypeClass, Sint32TypeClass];
+  let HasGroup = 1;
+}
+
 //===----------------------------------------------------------------------===//
 // Comparison opcodes.
 //===----------------------------------------------------------------------===//
 
 class EqualityOpcode : Opcode {
   let Types = [AllTypeClass];
   let HasGroup = 1;
 }

clang/test/AST/Interp/literals.cpp

 // RUN: %clang_cc1 -fexperimental-new-constant-interpreter -std=c++11 -verify %s
+// RUN: %clang_cc1 -std=c++11 -verify=ref %s
 
 static_assert(true, "");
-static_assert(false, ""); // expected-error{{failed}}
+static_assert(false, ""); // expected-error{{failed}} ref-error{{failed}}
 static_assert(nullptr == nullptr, "");
 static_assert(1 == 1, "");
-static_assert(1 == 3, ""); // expected-error{{failed}}
+static_assert(1 == 3, ""); // expected-error{{failed}} ref-error{{failed}}
 
 constexpr int number = 10;
 static_assert(number == 10, "");
-static_assert(number != 10, ""); // expected-error{{failed}}
+static_assert(number != 10, ""); // expected-error{{failed}} \
+                                 // ref-error{{failed}} \
+                                 // ref-note{{evaluates to}}
 
 constexpr bool getTrue() { return true; }
 constexpr bool getFalse() { return false; }
 constexpr void* getNull() { return nullptr; }
+
+constexpr int neg(int m) { return -m; }
+constexpr bool inv(bool b) { return !b; }
+
+static_assert(12, "");
+static_assert(12 == -(-(12)), "");
+static_assert(!false, "");
+static_assert(!!true, "");
+static_assert(!!true == !false, "");
+static_assert(true == 1, "");
+static_assert(false == 0, "");
+static_assert(!5 == false, "");
+static_assert(!0, "");
+static_assert(-true, "");
+static_assert(-false, ""); //expected-error{{failed}} ref-error{{failed}}