From 70c2ca58af171dbacad3ffb1f6dd3d6d313c5e8c Mon Sep 17 00:00:00 2001 From: abdul124 Date: Mon, 12 Aug 2024 17:31:43 +0800 Subject: [PATCH] firmware/ksupport: improve comments and syscall name --- artiq/compiler/embedding.py | 6 +++--- artiq/coredevice/core.py | 2 +- artiq/coredevice/exceptions.py | 6 +++--- artiq/firmware/ksupport/api.rs | 3 ++- artiq/firmware/ksupport/eh_artiq.rs | 11 ++++++++--- artiq/test/coredevice/test_exceptions.py | 10 +++++----- 6 files changed, 22 insertions(+), 16 deletions(-) diff --git a/artiq/compiler/embedding.py b/artiq/compiler/embedding.py index 6003d5845..4e19303b0 100644 --- a/artiq/compiler/embedding.py +++ b/artiq/compiler/embedding.py @@ -57,9 +57,9 @@ class EmbeddingMap: self.str_forward_map = {} self.str_reverse_map = {} - # Keep this list of exceptions in sync with `EXCEPTION_ID_LOOKUP` in `artiq.firmware.ksupport.eh_artiq`` - # The exceptions declared here should be defined in `artiq.coredeive.exceptions`` - # Without sync, test cases in artiq.test.coredevice.test_exceptions would fail + # Keep this list of exceptions in sync with `EXCEPTION_ID_LOOKUP` in `artiq::firmware::ksupport::eh_artiq` + # The exceptions declared here must be defined in `artiq.coredevice.exceptions` + # Verify synchronization by running the test cases in `artiq.test.coredevice.test_exceptions` self.preallocate_runtime_exception_names([ "RTIOUnderflow", "RTIOOverflow", diff --git a/artiq/coredevice/core.py b/artiq/coredevice/core.py index e7b6b4874..b0c595e5c 100644 --- a/artiq/coredevice/core.py +++ b/artiq/coredevice/core.py @@ -53,7 +53,7 @@ def rtio_get_counter() -> TInt64: raise NotImplementedError("syscall not simulated") @syscall -def raise_exception(id: TInt32) -> TNone: +def test_exception_id_sync(id: TInt32) -> TNone: raise NotImplementedError("syscall not simulated") class Core: diff --git a/artiq/coredevice/exceptions.py b/artiq/coredevice/exceptions.py index 5cf3d3b91..3ec699b07 100644 --- a/artiq/coredevice/exceptions.py +++ b/artiq/coredevice/exceptions.py @@ -7,10 +7,10 @@ from artiq import __artiq_dir__ as artiq_dir from artiq.coredevice.runtime import source_loader """ -This file should provide class definition for all the exceptions declared in `EmbeddingMap` in artiq.compiler.embedding +This file provides class definition for all the exceptions declared in `EmbeddingMap` in `artiq.compiler.embedding` -For python builtin exceptions, use the `builtins` module -For artiq specific exceptions, inherit from `Exception` class +For Python builtin exceptions, use the `builtins` module +For ARTIQ specific exceptions, inherit from `Exception` class """ AssertionError = builtins.AssertionError diff --git a/artiq/firmware/ksupport/api.rs b/artiq/firmware/ksupport/api.rs index 7b1fa4ec2..63e94cdd2 100644 --- a/artiq/firmware/ksupport/api.rs +++ b/artiq/firmware/ksupport/api.rs @@ -171,7 +171,8 @@ static mut API: &'static [(&'static str, *const ())] = &[ /* * syscall for unit tests * Used in `artiq.tests.coredevice.test_exceptions.ExceptionTest.test_raise_exceptions_kernel` - * `EmbeddingMap` (`artiq.compiler.embedding`) and `EXCEPTION_ID_LOOKUP` (`artiq.firmware.ksupport.eh_artiq`) + * This syscall checks that the exception IDs used in the Python `EmbeddingMap` (in `artiq.compiler.embedding`) + * match the `EXCEPTION_ID_LOOKUP` defined in the firmware (`artiq::firmware::ksupport::eh_artiq`) */ api!(test_exception_id_sync = ::eh_artiq::test_exception_id_sync) ]; diff --git a/artiq/firmware/ksupport/eh_artiq.rs b/artiq/firmware/ksupport/eh_artiq.rs index 44dda1015..b47e61a20 100644 --- a/artiq/firmware/ksupport/eh_artiq.rs +++ b/artiq/firmware/ksupport/eh_artiq.rs @@ -365,15 +365,20 @@ pub fn get_exception_id(name: &str) -> u32 { unimplemented!("unallocated internal exception id") } -// Performs a reverse lookup on `EXCEPTION_ID_LOOKUP` -// Unconditionally raises an exception given its id +/// Takes as input exception id from host +/// Generates a new exception with: +/// * `id` set to `exn_id` +/// * `message` set to corresponding exception name from `EXCEPTION_ID_LOOKUP` +/// +/// The message is matched on host to ensure correct exception is being referred +/// This test checks the synchronization of exception ids for runtime errors #[no_mangle] #[unwind(allowed)] pub extern fn test_exception_id_sync(exn_id: u32) { let message = EXCEPTION_ID_LOOKUP .iter() .find_map(|&(name, id)| if id == exn_id { Some(name) } else { None }) - .unwrap_or("Unknown exception ID"); + .unwrap_or("unallocated internal exception id"); let exn = Exception { id: exn_id, diff --git a/artiq/test/coredevice/test_exceptions.py b/artiq/test/coredevice/test_exceptions.py index 26d6f8ca3..58c297061 100644 --- a/artiq/test/coredevice/test_exceptions.py +++ b/artiq/test/coredevice/test_exceptions.py @@ -4,16 +4,16 @@ import artiq.coredevice.exceptions as exceptions from artiq.experiment import * from artiq.test.hardware_testbench import ExperimentCase from artiq.compiler.embedding import EmbeddingMap -from artiq.coredevice.core import raise_exception +from artiq.coredevice.core import test_exception_id_sync """ Test sync in exceptions raised between host and kernel -Check artiq.compiler.embedding and artiq.frontend.ksupport.eh_artiq +Check `artiq.compiler.embedding` and `artiq::firmware::ksupport::eh_artiq` Considers the following two cases: 1) Exception raised on kernel and passed to host - 2) Exception raised in host function called from kernel -Ensures integirty of exceptions is maintained as it passes between kernel and host + 2) Exception raised in a host function called from kernel +Ensures same exception is raised on both kernel and host in either case """ exception_names = EmbeddingMap().str_reverse_map @@ -35,7 +35,7 @@ class _TestExceptionSync(EnvExperiment): @kernel def raise_exception_kernel(self, id): - raise_exception(id) + test_exception_id_sync(id) class ExceptionTest(ExperimentCase):