Follow-up to PR#4266 - Addressing review comments#4275
Follow-up to PR#4266 - Addressing review comments#4275khalatepradnya wants to merge 2 commits intoNVIDIA:mainfrom
Conversation
* Remove redundant check * Fix the pattern(s) by pre-declaring functions so we don't have multiple threads try to update a ModuleOp at the same time Signed-off-by: Pradnya Khalate <pkhalate@nvidia.com>
|
CUDA Quantum Docs Bot: A preview of the documentation can be found here. |
| cudaq::opt::factory::createLLVMFunctionSymbol( | ||
| cudaq::opt::QIRArrayGetSize, IntegerType::get(context, 64), | ||
| {cudaq::opt::getArrayType(context)}, op); |
There was a problem hiding this comment.
We should use the intrinsics loading machinery for this if we can. It will properly deal with redundant declarations, etc.
There was a problem hiding this comment.
Yes, I tried using IRBuilder::loadIntrinsic here but it crashes with Loading a dialect (cc) while in a multi-threaded execution context. Adding cc and func to dependentDialects fixes that, but then applyFullConversion hits "redefinition of symbol". I think because createLLVMFunctionSymbol misses the func.func from loadIntrinsic and creates a duplicate.
A potential fix would be to widen createLLVMFunctionSymbol such that it also finds func::FuncOp declarations, but that change is pervasive, and I wasn't sure whether it is right approach.
Any pointers?
|
CUDA Quantum Docs Bot: A preview of the documentation can be found here. |
2 participants
quake.measurements_sizeop #4266