[clang][bytcode] Convert Fixed Point values to target semantics... (l…

…lvm#110411) ... after a binary operation. The Result of the operation is in the common semantics of RHS and LHS, so we need to convert that to the semantics of the BinaryOperator expression.
ROCm · Sep 29, 2024 · 1714b11 · 1714b11
1 parent c2a37e4
commit 1714b11
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 10 deletions.
diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp
@@ -1502,25 +1502,41 @@ bool Compiler<Emitter>::VisitFixedPointBinOp(const BinaryOperator *E) {
   assert(LHS->getType()->isFixedPointType() ||
          RHS->getType()->isFixedPointType());
 
+  auto LHSSema = Ctx.getASTContext().getFixedPointSemantics(LHS->getType());
+  auto RHSSema = Ctx.getASTContext().getFixedPointSemantics(RHS->getType());
+
   if (!this->visit(LHS))
     return false;
   if (!LHS->getType()->isFixedPointType()) {
-    auto Sem = Ctx.getASTContext().getFixedPointSemantics(LHS->getType());
     uint32_t I;
-    std::memcpy(&I, &Sem, sizeof(Sem));
+    std::memcpy(&I, &LHSSema, sizeof(llvm::FixedPointSemantics));
     if (!this->emitCastIntegralFixedPoint(classifyPrim(LHS->getType()), I, E))
       return false;
   }
+
   if (!this->visit(RHS))
     return false;
   if (!RHS->getType()->isFixedPointType()) {
-    auto Sem = Ctx.getASTContext().getFixedPointSemantics(RHS->getType());
     uint32_t I;
-    std::memcpy(&I, &Sem, sizeof(Sem));
+    std::memcpy(&I, &RHSSema, sizeof(llvm::FixedPointSemantics));
     if (!this->emitCastIntegralFixedPoint(classifyPrim(RHS->getType()), I, E))
       return false;
   }
 
+  // Convert the result to the target semantics.
+  auto ConvertResult = [&](bool R) -> bool {
+    if (!R)
+      return false;
+    auto ResultSema = Ctx.getASTContext().getFixedPointSemantics(E->getType());
+    auto CommonSema = LHSSema.getCommonSemantics(RHSSema);
+    if (ResultSema != CommonSema) {
+      uint32_t I;
+      std::memcpy(&I, &ResultSema, sizeof(ResultSema));
+      return this->emitCastFixedPoint(I, E);
+    }
+    return true;
+  };
+
   switch (E->getOpcode()) {
   case BO_EQ:
     return this->emitEQFixedPoint(E);
@@ -1537,7 +1553,7 @@ bool Compiler<Emitter>::VisitFixedPointBinOp(const BinaryOperator *E) {
     return this->emitGEFixedPoint(E);
 #endif
   case BO_Add:
-    return this->emitAddFixedPoint(E);
+    return ConvertResult(this->emitAddFixedPoint(E));
 
   default:
     return this->emitInvalid(E);

diff --git a/clang/lib/AST/ByteCode/Interp.h b/clang/lib/AST/ByteCode/Interp.h
@@ -2177,7 +2177,8 @@ inline bool CastFixedPoint(InterpState &S, CodePtr OpPC, uint32_t FPS) {
           E->getExprLoc(), diag::warn_fixedpoint_constant_overflow)
           << Result.toDiagnosticString(S.getASTContext()) << E->getType();
     }
-    S.CCEDiag(E, diag::note_constexpr_overflow) << Result << E->getType();
+    S.CCEDiag(E, diag::note_constexpr_overflow)
+        << Result.toDiagnosticString(S.getASTContext()) << E->getType();
     if (!S.noteUndefinedBehavior())
       return false;
   }

diff --git a/clang/test/AST/ByteCode/fixed-point.cpp b/clang/test/AST/ByteCode/fixed-point.cpp
@@ -42,10 +42,8 @@ namespace BinOps {
   static_assert(1 + A == 14.0k);
   static_assert((A + A) == 26);
 
-  /// FIXME: Conversion between fixed point semantics.
-  static_assert(A + 100000 == 14.0k); // expected-error {{static assertion failed}} \
-                                      // ref-error {{is not an integral constant expression}} \
-                                      // ref-note {{is outside the range of representable values}}
+  static_assert(A + 100000 == 14.0k); // both-error {{is not an integral constant expression}} \
+                                      // both-note {{is outside the range of representable values}}
 }
 
 namespace FixedPointCasts {