[clang][Interp] Fix compound assign operator types

[tbaeder]

Summary

Just like we do (or will do) for floating types, we need to take into
acocunt that the LHSComputationType, ResultType and type of the
expression (what we ultimately store) might be different.

Do this by emitting cast ops before and after doing the computation.

This fixes the test failures introduced by
490e8214fca48824beda8b508d6d6bbbf3d8d9a7 on big endian machines.

[tbaeder]

The casts emitted are similar to https://reviews.llvm.org/D140377 now.

[tbaeder]

I know it hasn't been a week yet, but can someone take a look at this? It would unblock pushing more of the approved patches.

[shafik]

LGTM

[tbaeder]

Thanks!

[steakhal]

Hi @tbaeder, I was looking for commits that mentions "fix" "assertion" or "crash" in the title, that are part of the main branch but not backported to release/16.x to be eventually released as clang-16.
I wonder what's the status of this and the related patches given that the commit that the summary mentions (490e8214fca48824beda8b508d6d6bbbf3d8d9a7) is present in release/16.x. I can also see that quite a few revert commits are also related to Interp.
It's probably the case that the mentioned change was removed from clang-16 by f6ea1af9a4b71d27de2dde629224af1220c5c85b.

Could you please confirm that the changes around Interp are consistent and no patches are missing from the release/16.x just to be sure?

[tbaeder]

All the interpreter work is experimental so it doesn't really need to be in the release branch. This should all be fine, thanks for asking.

clang/lib/AST/Interp/ByteCodeExprGen.cpp

bool ByteCodeExprGen<Emitter>::VisitCharacterLiteral(
   return this->emitConst(E->getValue(), E);
 }
 
 template <class Emitter>
 bool ByteCodeExprGen<Emitter>::VisitCompoundAssignOperator(
     const CompoundAssignOperator *E) {
   const Expr *LHS = E->getLHS();
   const Expr *RHS = E->getRHS();
-  std::optional<PrimType> LT = classify(E->getLHS()->getType());
-  std::optional<PrimType> RT = classify(E->getRHS()->getType());
+  std::optional<PrimType> LHSComputationT =
+      classify(E->getComputationLHSType());
+  std::optional<PrimType> LT = classify(LHS->getType());
+  std::optional<PrimType> RT = classify(E->getComputationResultType());
+  std::optional<PrimType> ResultT = classify(E->getType());
 
-  if (!LT || !RT)
+  if (!LT || !RT || !ResultT || !LHSComputationT)
     return false;
 
   assert(!E->getType()->isPointerType() &&
          "Support pointer arithmethic in compound assignment operators");
 
   // Get LHS pointer, load its value and get RHS value.
   if (!visit(LHS))
     return false;
   if (!this->emitLoad(*LT, E))
     return false;
+  // If necessary, cast LHS to its computation type.
+  if (*LT != *LHSComputationT) {
+    if (!this->emitCast(*LT, *LHSComputationT, E))
+      return false;
+  }
+
   if (!visit(RHS))
     return false;
 
   // Perform operation.
   switch (E->getOpcode()) {
   case BO_AddAssign:
-    if (!this->emitAdd(*LT, E))
+    if (!this->emitAdd(*LHSComputationT, E))
       return false;
     break;
   case BO_SubAssign:
-    if (!this->emitSub(*LT, E))
+    if (!this->emitSub(*LHSComputationT, E))
       return false;
     break;
 
   case BO_MulAssign:
   case BO_DivAssign:
   case BO_RemAssign:
+
   case BO_ShlAssign:
-    if (!this->emitShl(*LT, *RT, E))
+    if (!this->emitShl(*LHSComputationT, *RT, E))
       return false;
     break;
   case BO_ShrAssign:
-    if (!this->emitShr(*LT, *RT, E))
+    if (!this->emitShr(*LHSComputationT, *RT, E))
       return false;
     break;
   case BO_AndAssign:
   case BO_XorAssign:
   case BO_OrAssign:
   default:
     llvm_unreachable("Unimplemented compound assign operator");
   }
 
+  // And now cast from LHSComputationT to ResultT.
+  if (*ResultT != *LHSComputationT) {
+    if (!this->emitCast(*LHSComputationT, *ResultT, E))
+      return false;
+  }
+
   // And store the result in LHS.
   if (DiscardResult)
-    return this->emitStorePop(*LT, E);
-  return this->emitStore(*LT, E);
+    return this->emitStorePop(*ResultT, E);
+  return this->emitStore(*ResultT, E);
 }
 
 template <class Emitter> bool ByteCodeExprGen<Emitter>::discard(const Expr *E) {
   OptionScope<Emitter> Scope(this, /*NewDiscardResult=*/true);
   return this->Visit(E);
 }
 
 template <class Emitter>