qir-alliance / qcor

C++ compiler for heterogeneous quantum-classical computing built on Clang and XACC
http://docs.aide-qc.org
MIT License
97 stars 39 forks source link

OPENQASM3: Why does array quantum declarations don't compute the size expression? #218

Closed moar55 closed 3 years ago

moar55 commented 3 years ago

I noticed that when declaring a quantum array (either by qubit or qreg), if the size of the array is provided as an expression the size is allocated with the first element of the expression. I found this in quantum_types_handler.cpp: here size = std::stoi(exp_list->expression(0)->getText()); and here exp_generator.visit(exp_list->expression(0)); From what I see it's valid syntax to have: qubit q1[4*3];. Is only taking the first element of the expression an intended design decision?

amccaskey commented 3 years ago

Nope that is a bug.

And its interesting - std::stoi() converts '4*3' to 4, which is not something I expected. Hence the catch block which will properly handle the expression.

amccaskey commented 3 years ago

Fix would be to change the try { ... } catch(...) {...} around the std::stoi() call, to instead be an if {} else{} based on ScopedSymbolTable::try_evaluate_constant_integer_expression( const std::string expr_str)

     auto opt_size = symbol_table.try_evaluate_constant_integer_expression(
          exp_list->expression(0)->getText());
      if (opt_size.has_value()) {
        size = opt_size.value();
      } else {
        // check if this is a constant expression
        qasm3_expression_generator exp_generator(builder, symbol_table,
                                                 file_name);
        exp_generator.visit(exp_list->expression(0));
        auto arg = exp_generator.current_value;

        if (auto constantOp = arg.getDefiningOp<mlir::ConstantOp>()) {
          if (constantOp.getValue().isa<mlir::IntegerAttr>()) {
            size = constantOp.getValue().cast<mlir::IntegerAttr>().getInt();
          } else {
            printErrorMessage(
                "This variable qubit size must be a constant integer.");
          }
        } else {
          size = symbol_table.get_global_constant<int64_t>(
              exp_list->expression(0)->getText());
        }
      }
amccaskey commented 3 years ago

Fix pushed in https://github.com/ORNL-QCI/qcor/commit/5966fd958f924da9d7d8cd63604f0faf24955fa7