* Properly(?) return measure_result type - may need clean up

khalatepradnya · khalatepradnya · commit 45b8c68986ec · 2026-02-05T22:31:56.000-08:00
- Factory and Marshal
* Can use `auto` now
* NVQIR function for logging result record seems to have issues

Signed-off-by: Pradnya Khalate &lt;pkhalate@nvidia.com&gt;
diff --git a/lib/Frontend/nvqpp/ASTBridge.cpp b/lib/Frontend/nvqpp/ASTBridge.cpp
@@ -53,7 +53,7 @@ listReachableFunctions(clang::CallGraphNode *cgn) {
 // Does `ty` refer to a Quake quantum type? This also checks custom recursive
 // types. It does not check builtin recursive types; e.g., `!llvm.ptr<T>`.
 static bool isQubitType(Type ty) {
-  if (quake::isQuakeType(ty))
+  if (quake::isQuantumType(ty))
     return true;
   // FIXME: next if case is a bug.
   if (auto vecTy = dyn_cast<cudaq::cc::StdvecType>(ty))
diff --git a/lib/Frontend/nvqpp/ConvertExpr.cpp b/lib/Frontend/nvqpp/ConvertExpr.cpp
@@ -99,6 +99,12 @@ maybeUnpackOperands(OpBuilder &builder, Location loc, ValueRange operands,
   return std::make_pair(targets, SmallVector<Value>{});
 }
 
+static Value emitDiscriminate(OpBuilder &builder, Location loc, Value val) {
+  if (isa<quake::MeasureType>(val.getType()))
+    return builder.create<quake::DiscriminateOp>(loc, builder.getI1Type(), val);
+  return val;
+}
+
 namespace {
 // Type used to specialize the buildOp function. This extends the cases below by
 // prefixing a single parameter value to the list of arguments for cases 1
@@ -637,11 +643,7 @@ bool QuakeBridgeVisitor::VisitCastExpr(clang::CastExpr *x) {
   }
   case clang::CastKind::CK_IntegralToFloating: {
     auto value = popValue();
-    // If source is `!quake.measure`, discriminate it first
-    if (isa<quake::MeasureType>(value.getType())) {
-      value = builder.create<quake::DiscriminateOp>(loc, builder.getI1Type(),
-                                                    value);
-    }
+    value = emitDiscriminate(builder, loc, value);
     auto mode =
         (x->getSubExpr()->getType()->isUnsignedIntegerOrEnumerationType())
             ? cudaq::cc::CastOpMode::Unsigned
@@ -651,24 +653,14 @@ bool QuakeBridgeVisitor::VisitCastExpr(clang::CastExpr *x) {
   }
   case clang::CastKind::CK_IntegralToBoolean: {
     auto last = popValue();
-    // If the value is `!quake.measure`, discriminate it first
-    if (isa<quake::MeasureType>(last.getType())) {
-      last =
-          builder.create<quake::DiscriminateOp>(loc, builder.getI1Type(), last);
-      return pushValue(last);
-    }
+    last = emitDiscriminate(builder, loc, last);
     Value zero = builder.create<arith::ConstantIntOp>(loc, 0, last.getType());
     return pushValue(builder.create<arith::CmpIOp>(
         loc, arith::CmpIPredicate::ne, last, zero));
   }
   case clang::CastKind::CK_FloatingToBoolean: {
     auto last = popValue();
-    // If the value is `!quake.measure`, discriminate it first
-    if (isa<quake::MeasureType>(last.getType())) {
-      last =
-          builder.create<quake::DiscriminateOp>(loc, builder.getI1Type(), last);
-      return pushValue(last);
-    }
+    last = emitDiscriminate(builder, loc, last);
     Value zero = opt::factory::createFloatConstant(
         loc, builder, 0.0, cast<FloatType>(last.getType()));
     return pushValue(builder.create<arith::CmpFOp>(
@@ -687,7 +679,7 @@ bool QuakeBridgeVisitor::VisitCastExpr(clang::CastExpr *x) {
     auto i1Type = builder.getI1Type();
     // Handle conversion of `measure_result`
     if (isa<quake::MeasureType>(sub.getType())) {
-      auto i1Val = builder.create<quake::DiscriminateOp>(loc, i1Type, sub);
+      auto i1Val = emitDiscriminate(builder, loc, sub);
       // Convert to `int`
       if (isa<IntegerType>(castToTy))
         return pushValue(
@@ -860,12 +852,8 @@ bool QuakeBridgeVisitor::VisitBinaryOperator(clang::BinaryOperator *x) {
     rhs = maybeLoadValue(rhs);
     lhs = maybeLoadValue(lhs);
     // Discriminate measure types before comparison
-    if (isa<quake::MeasureType>(lhs.getType()))
-      lhs =
-          builder.create<quake::DiscriminateOp>(loc, builder.getI1Type(), lhs);
-    if (isa<quake::MeasureType>(rhs.getType()))
-      rhs =
-          builder.create<quake::DiscriminateOp>(loc, builder.getI1Type(), rhs);
+    lhs = emitDiscriminate(builder, loc, lhs);
+    rhs = emitDiscriminate(builder, loc, rhs);
     // Floating point comparison?
     if (isa<FloatType>(lhs.getType())) {
       arith::CmpFPredicate pred;
@@ -945,10 +933,8 @@ bool QuakeBridgeVisitor::VisitBinaryOperator(clang::BinaryOperator *x) {
   rhs = maybeLoadValue(rhs);
   lhs = maybeLoadValue(lhs);
   // Discriminate measure types before arithmetic
-  if (isa<quake::MeasureType>(lhs.getType()))
-    lhs = builder.create<quake::DiscriminateOp>(loc, builder.getI1Type(), lhs);
-  if (isa<quake::MeasureType>(rhs.getType()))
-    rhs = builder.create<quake::DiscriminateOp>(loc, builder.getI1Type(), rhs);
+  lhs = emitDiscriminate(builder, loc, lhs);
+  rhs = emitDiscriminate(builder, loc, rhs);
   castToSameType(builder, loc, x->getLHS()->getType().getTypePtrOrNull(), lhs,
                  x->getRHS()->getType().getTypePtrOrNull(), rhs);
   switch (x->getOpcode()) {
@@ -1036,10 +1022,7 @@ bool QuakeBridgeVisitor::TraverseConditionalOperator(
   if (!TraverseStmt(x->getCond()))
     return false;
   auto condVal = popValue();
-  // Discriminate if condition is `!quake.measure`
-  if (isa<quake::MeasureType>(condVal.getType()))
-    condVal = builder.create<quake::DiscriminateOp>(loc, builder.getI1Type(),
-                                                    condVal);
+  condVal = emitDiscriminate(builder, loc, condVal);
   Type resultTy = builder.getI64Type();
 
   // Create shared lambda for the x->getTrueExpr() and x->getFalseExpr()
@@ -1622,12 +1605,8 @@ bool QuakeBridgeVisitor::VisitCallExpr(clang::CallExpr *x) {
       if (isa<cc::PointerType>(rhs.getType()))
         rhs = builder.create<cc::LoadOp>(loc, rhs);
       // Discriminate measure types
-      if (isa<quake::MeasureType>(lhs.getType()))
-        lhs = builder.create<quake::DiscriminateOp>(loc, builder.getI1Type(),
-                                                    lhs);
-      if (isa<quake::MeasureType>(rhs.getType()))
-        rhs = builder.create<quake::DiscriminateOp>(loc, builder.getI1Type(),
-                                                    rhs);
+      lhs = emitDiscriminate(builder, loc, lhs);
+      rhs = emitDiscriminate(builder, loc, rhs);
       // Choose predicate based on operator
       auto pred = (opKind == clang::OO_EqualEqual) ? arith::CmpIPredicate::eq
                                                    : arith::CmpIPredicate::ne;
diff --git a/lib/Optimizer/Builder/Factory.cpp b/lib/Optimizer/Builder/Factory.cpp
@@ -71,6 +71,11 @@ Type genBufferType(Type ty) {
     assert(!cudaq::cc::isDynamicType(ty) && "must be a type of static extent");
     return ty;
   }
+  if (isa<quake::MeasureType>(ty)) {
+    auto i32Ty = IntegerType::get(ctx, 32);
+    auto i64Ty = IntegerType::get(ctx, 64);
+    return cudaq::cc::StructType::get(ctx, {i32Ty, i64Ty});
+  }
   return ty;
 }
 
@@ -430,6 +435,13 @@ Type factory::convertToHostSideType(Type ty, ModuleOp mod) {
     return cc::PointerType::get(factory::stlVectorType(
         IntegerType::get(ctx, /*FIXME sizeof a pointer?*/ 64)));
   }
+  if (isa<quake::MeasureType>(ty)) {
+    auto *ctx = ty.getContext();
+    auto i32Ty = IntegerType::get(ctx, 32);
+    auto i64Ty = IntegerType::get(ctx, 64);
+    // Return the `measure_result` struct {int result, size_t uniqueId}
+    return cc::StructType::get(ctx, {i32Ty, i64Ty});
+  }
   return ty;
 }
 
@@ -644,7 +656,7 @@ FunctionType factory::toHostSideFuncType(FunctionType funcTy, bool addThisPtr,
       hasSRet = true;
     } else {
       assert(funcTy.getNumResults() == 1);
-      resultTy = funcTy.getResult(0);
+      resultTy = convertToHostSideType(funcTy.getResult(0), module);
     }
   }
   // If this kernel is a plain old function or a static member function, we
diff --git a/lib/Optimizer/Builder/Intrinsics.cpp b/lib/Optimizer/Builder/Intrinsics.cpp
@@ -457,6 +457,9 @@ static constexpr IntrinsicCode intrinsicTable[] = {
 )#"},
     {cudaq::opt::QIRIntegerRecordOutput, {}, R"#(
   func.func private @__quantum__rt__int_record_output(i64, !cc.ptr<i8>)
+)#"},
+    {cudaq::opt::QIRRecordOutput, {}, R"#(
+  func.func private @__quantum__rt__result_record_output(!cc.ptr<!llvm.struct<"Result", opaque>>, !cc.ptr<i8>)
 )#"},
     {cudaq::opt::QIRTupleRecordOutput, {}, R"#(
   func.func private @__quantum__rt__tuple_record_output(i64, !cc.ptr<i8>)
diff --git a/lib/Optimizer/Builder/Marshal.cpp b/lib/Optimizer/Builder/Marshal.cpp
@@ -790,6 +790,9 @@ std::pair<bool, func::FuncOp> cudaq::opt::marshal::lookupHostEntryPointFunc(
     // No host entry point needed.
     return {false, func::FuncOp{}};
   }
+  if (!funcOp->hasAttr(cudaq::entryPointAttrName)) {
+    return {false, func::FuncOp{}};
+  }
   if (auto *decl = module.lookupSymbol(mangledEntryPointName))
     if (auto func = dyn_cast<func::FuncOp>(decl)) {
       func.eraseBody();
diff --git a/lib/Optimizer/CodeGen/ConvertCCToLLVM.cpp b/lib/Optimizer/CodeGen/ConvertCCToLLVM.cpp
@@ -81,19 +81,19 @@ void cudaq::opt::populateCCTypeConversions(LLVMTypeConverter *converter) {
     return LLVM::LLVMStructType::getLiteral(type.getContext(), members,
                                             type.getPacked());
   });
-}
-
-std::size_t cudaq::opt::getDataSize(llvm::DataLayout &dataLayout, Type ty) {
-  LLVMTypeConverter converter(ty.getContext());
-  cudaq::opt::populateCCTypeConversions(&converter);
-  auto llvmDialectTy = converter.convertType(ty);
   // `measure_result` -> struct conversion for size calculation
-  converter.addConversion([](quake::MeasureType type) -> Type {
+  converter->addConversion([](quake::MeasureType type) -> Type {
     auto ctx = type.getContext();
     auto i32Ty = IntegerType::get(ctx, 32);
     auto i64Ty = IntegerType::get(ctx, 64);
     return LLVM::LLVMStructType::getLiteral(ctx, {i32Ty, i64Ty});
   });
+}
+
+std::size_t cudaq::opt::getDataSize(llvm::DataLayout &dataLayout, Type ty) {
+  LLVMTypeConverter converter(ty.getContext());
+  cudaq::opt::populateCCTypeConversions(&converter);
+  auto llvmDialectTy = converter.convertType(ty);
   llvm::LLVMContext context;
   LLVM::TypeToLLVMIRTranslator translator(context);
   auto llvmTy = translator.translateType(llvmDialectTy);
@@ -104,13 +104,6 @@ std::size_t cudaq::opt::getDataOffset(llvm::DataLayout &dataLayout, Type ty,
                                       std::size_t off) {
   LLVMTypeConverter converter(ty.getContext());
   cudaq::opt::populateCCTypeConversions(&converter);
-  // `measure_result` -> struct conversion for size calculation
-  converter.addConversion([](quake::MeasureType type) -> Type {
-    auto ctx = type.getContext();
-    auto i32Ty = IntegerType::get(ctx, 32);
-    auto i64Ty = IntegerType::get(ctx, 64);
-    return LLVM::LLVMStructType::getLiteral(ctx, {i32Ty, i64Ty});
-  });
   auto llvmDialectTy = converter.convertType(ty);
   llvm::LLVMContext context;
   LLVM::TypeToLLVMIRTranslator translator(context);
diff --git a/lib/Optimizer/CodeGen/ReturnToOutputLog.cpp b/lib/Optimizer/CodeGen/ReturnToOutputLog.cpp
@@ -170,6 +170,22 @@ class ReturnRewrite : public OpRewritePattern<cudaq::cc::LogOutputOp> {
                                         cudaq::opt::QIRRecordOutput,
                                         ArrayRef<Value>{val, label});
         })
+        .Case([&](cudaq::cc::PointerType ptrTy) {
+          // Check if this is a pointer to %Result (converted MeasureType)
+          if (auto structTy =
+                  dyn_cast<LLVM::LLVMStructType>(ptrTy.getElementType()))
+            if (structTy.isIdentified() && structTy.getName() == "Result") {
+              // Handle as measure result
+              std::string labelStr = "result";
+              if (prefix)
+                labelStr = prefix->str();
+              Value label = makeLabel(loc, rewriter, labelStr);
+              rewriter.create<func::CallOp>(loc, TypeRange{},
+                                            cudaq::opt::QIRRecordOutput,
+                                            ArrayRef<Value>{val, label});
+              return;
+            }
+        })
         .Default([&](Type) {
           // If we reach here, we don't know how to handle this type.
           Value one = rewriter.create<arith::ConstantIntOp>(loc, 1, 64);
@@ -236,6 +252,11 @@ struct ReturnToOutputLogPass
       signalPassFailure();
       return;
     }
+    if (failed(irBuilder.loadIntrinsic(module, cudaq::opt::QIRRecordOutput))) {
+      module.emitError("could not load QIR result record output function.");
+      signalPassFailure();
+      return;
+    }
     if (failed(irBuilder.loadIntrinsic(module, cudaq::opt::QISTrap))) {
       module.emitError("could not load QIR trap function.");
       signalPassFailure();
diff --git a/runtime/common/RecordLogParser.cpp b/runtime/common/RecordLogParser.cpp
@@ -105,77 +105,6 @@ void cudaq::RecordLogParser::handleOutput(
   const std::string &recValue = entries[2];
   std::string recLabel = (entries.size() == 4) ? entries[3] : "";
   cudaq::trim(recLabel);
-  if (recType == "RESULT") {
-    // Sample-type QIR output, where we have an array of `RESULT` per shot. For
-    // example,
-    //  START
-    //  OUTPUT    RESULT  1       r00000
-    //  ....
-    //  OUTPUT    RESULT  1       r00009
-    //  END       0
-
-    currentOutput = OutputType::RESULT;
-    const bool isUninitializedContainer =
-        (containerMeta.m_type == ContainerType::NONE) ||
-        (containerMeta.m_type == ContainerType::ARRAY &&
-         containerMeta.elementCount == 0);
-    if (isUninitializedContainer) {
-      // NOTE: This is a temporary workaround until all backends consistently
-      // use the new transformation pass that wraps result records inside an
-      // array record output. For now, we permit "naked" RESULT records, i.e.,
-      // if the QIR produced by a sampled kernel emits a sequence of RESULT
-      // records without enclosing them in an ARRAY, we interpret them
-      // collectively as an array of results.
-      // NOTE: This assumption prevents us from correctly supporting `run` with
-      // `qir-base` profile.
-      containerMeta.m_type = ContainerType::ARRAY;
-      containerMeta.elementCount =
-          std::stoul(metadata[ResultCountMetadataName]);
-      containerMeta.arrayType = "i1";
-      preallocateArray();
-    }
-
-    // Note: For ordered schema, we expect the results are sequential in the
-    // same order that mz operations are called. This may include results in
-    // named registers (specified in kernel code) and other auto-generated
-    // register names. If index cannot be extracted from the label, we fall back
-    // to using this mechanism.
-    auto idxLabel = std::to_string(containerMeta.processedElements);
-
-    // Get the index from the label, if feasible.
-    /// TODO: The `sample` API should be updated to not allow explicit
-    /// measurement operations in the kernel when targeting hardware backends.
-    // Until then, we handle both cases here - auto-generated labels like
-    // r00000, r00001, ... and named results like result%0, result%1, ...
-    if (!recLabel.empty()) {
-      std::size_t percentPos = recLabel.find('%');
-      if (percentPos != std::string::npos) {
-        idxLabel = recLabel.substr(percentPos + 1);
-      }
-      // This logic is fragile; for example user may have only one mz assigned
-      // to variable like r00001 and it will be interpreted as index 1, and
-      // cause `Array index out of bounds` error. The proper fix is to disallow
-      // explicit mz operations in sampled kernels. Also, `run` is appropriate
-      // for getting sub-register results.
-      else if (recLabel.size() == 6 && recLabel[0] == 'r') {
-        // check that the last 5 characters are all digits
-        bool allDigits = true;
-        for (std::size_t i = 1; i < 6; ++i) {
-          if (recLabel[i] < '0' || recLabel[i] > '9') {
-            allDigits = false;
-            break;
-          }
-        }
-        if (allDigits) {
-          idxLabel = recLabel.substr(1);
-        }
-      }
-    }
-
-    processArrayEntry(recValue, fmt::format("[{}]", idxLabel));
-    containerMeta.processedElements++;
-    return;
-  }
   if (recType == "ARRAY") {
     containerMeta.m_type = ContainerType::ARRAY;
     containerMeta.elementCount = std::stoul(recValue);
@@ -196,6 +125,8 @@ void cudaq::RecordLogParser::handleOutput(
     }
     return;
   }
+  if (recType == "RESULT")
+    currentOutput = OutputType::RESULT;
   if (recType == "BOOL")
     currentOutput = OutputType::BOOL;
   else if (recType == "INT")
diff --git a/runtime/nvqir/NVQIR.cpp b/runtime/nvqir/NVQIR.cpp
@@ -675,6 +675,23 @@ void __quantum__rt__result_record_output(Result *r, int8_t *name) {
   if (ctx && ctx->name == "run") {
 
     std::string regName(reinterpret_cast<const char *>(name));
+
+    // Base profile - value stored in measRes2Val
+    auto iter = measRes2Val.find(r);
+    if (iter != measRes2Val.end()) {
+      quantumRTGenericRecordOutput("RESULT", (iter->second ? 1 : 0),
+                                   regName.c_str());
+      return;
+    }
+
+    // Full QIR - r is ResultOne or ResultZero (measurement already happened)
+    if (r == ResultOne || r == ResultZero) {
+      bool val = (r == ResultOne);
+      quantumRTGenericRecordOutput("RESULT", (val ? 1 : 0), regName.c_str());
+      return;
+    }
+
+    // Fallback - use qubit mapping and re-measure (legacy behavior)
     auto qI = qubitToSizeT(measRes2QB[r]);
     auto b = nvqir::getCircuitSimulatorInternal()->mz(qI, regName);
     quantumRTGenericRecordOutput("RESULT", (b ? 1 : 0), regName.c_str());
diff --git a/runtime/nvqir/QIRTypes.h b/runtime/nvqir/QIRTypes.h
@@ -89,6 +89,7 @@ void __nvqpp_cleanup_arrays();
 }
 
 // Results
+/// FIXME: What should this be?
 using Result = bool;
 static const Result ResultZeroVal = false;
 static const Result ResultOneVal = true;
diff --git a/targettests/Kernel/inline-qpu-func.cpp b/targettests/Kernel/inline-qpu-func.cpp
diff --git a/targettests/execution/conditional_run.cpp b/targettests/execution/conditional_run.cpp
diff --git a/targettests/execution/qubit_management_if_classical.cpp b/targettests/execution/qubit_management_if_classical.cpp

Original file line number	Diff line number	Diff line change
`@@ -71,6 +71,11 @@ Type genBufferType(Type ty) {`
`71`	`71`	`assert(!cudaq::cc::isDynamicType(ty) && "must be a type of static extent");`
`72`	`72`	`return ty;`
`73`	`73`	`}`
	`74`	`+ if (isa<quake::MeasureType>(ty)) {`
	`75`	`+ auto i32Ty = IntegerType::get(ctx, 32);`
	`76`	`+ auto i64Ty = IntegerType::get(ctx, 64);`
	`77`	`+ return cudaq::cc::StructType::get(ctx, {i32Ty, i64Ty});`
	`78`	`+ }`
`74`	`79`	`return ty;`
`75`	`80`	`}`
`76`	`81`
`@@ -430,6 +435,13 @@ Type factory::convertToHostSideType(Type ty, ModuleOp mod) {`
`430`	`435`	`return cc::PointerType::get(factory::stlVectorType(`
`431`	`436`	`IntegerType::get(ctx, /FIXME sizeof a pointer?/ 64)));`
`432`	`437`	`}`
	`438`	`+ if (isa<quake::MeasureType>(ty)) {`
	`439`	`+ auto *ctx = ty.getContext();`
	`440`	`+ auto i32Ty = IntegerType::get(ctx, 32);`
	`441`	`+ auto i64Ty = IntegerType::get(ctx, 64);`
	`442`	+ // Return the `measure_result` struct {int result, size_t uniqueId}
	`443`	`+ return cc::StructType::get(ctx, {i32Ty, i64Ty});`
	`444`	`+ }`
`433`	`445`	`return ty;`
`434`	`446`	`}`
`435`	`447`
`@@ -644,7 +656,7 @@ FunctionType factory::toHostSideFuncType(FunctionType funcTy, bool addThisPtr,`
`644`	`656`	`hasSRet = true;`
`645`	`657`	`} else {`
`646`	`658`	`assert(funcTy.getNumResults() == 1);`
`647`		`- resultTy = funcTy.getResult(0);`
	`659`	`+ resultTy = convertToHostSideType(funcTy.getResult(0), module);`
`648`	`660`	`}`
`649`	`661`	`}`
`650`	`662`	`// If this kernel is a plain old function or a static member function, we`
Original file line number	Diff line number	Diff line change
`@@ -89,6 +89,7 @@ void __nvqpp_cleanup_arrays();`
`89`	`89`	`}`
`90`	`90`
`91`	`91`	`// Results`
	`92`	`+/// FIXME: What should this be?`
`92`	`93`	`using Result = bool;`
`93`	`94`	`static const Result ResultZeroVal = false;`
`94`	`95`	`static const Result ResultOneVal = true;`