[sw, style] Replace LLVM-style X macro guidance
The new guidance is consistent with the far more common practice
throughout the project; we also eliminate the sole LLVM-style X macro.
Signed-off-by: Miguel Young de la Sota <mcyoung@google.com>
diff --git a/doc/rm/c_cpp_coding_style.md b/doc/rm/c_cpp_coding_style.md
index 4f1af11..22c076e 100644
--- a/doc/rm/c_cpp_coding_style.md
+++ b/doc/rm/c_cpp_coding_style.md
@@ -198,16 +198,21 @@
### X Macros
-In order to avoid repetitive definitions or statements, we allow the use of [X Macros](https://en.wikipedia.org/wiki/X_Macro) in our C and C++ code.
+In order to avoid repetitive definitions or statements, we allow the use of [X macros](https://en.wikipedia.org/wiki/X_Macro) in our C and C++ code.
Uses of X Macros should follow the following example, which uses this pattern in a switch definition:
```c
+#define MANY_FIELDS(X) \
+ X(1, 2, 3) \
+ X(4, 5, 6)
+
int get_field2(int identifier) {
+#define ITEM_(id_field, data_field1, data_field2) \
+ case id_field: \
+ return data_field2;
+
switch (identifier) {
-#define ITEM(id_field, data_field1, data_field2) \
- case id_field: \
- return data_field2;
-#include "path/to/item.def"
+ MANY_FIELDS(ITEM_)
default:
return 0;
}
@@ -215,49 +220,9 @@
```
This example expands to a case statement for each item, which returns the `data_field2` value where the passed in identifier matches `id_field`.
-The contents of the X Macro file from this example ("path/to/item.def") should look like:
-```c
-
-/**
- * \def ITEM(id_field, data_field1, data_field2)
- *
- * <Documentation about meaning of ITEM fields>
- */
-#ifndef ITEM
-#error ITEM(id_field, data_field1, data_field2) must be defined
-#endif
-
-ITEM(fields...)
-ITEM(fields...)
-
-#undef ITEM
-```
-
-These X Macro files must:
-
-* Have the extension `.def`.
- The file's basename should match the X Macro name.
- This is so we can easily identify that this is an X Macro, which will be used by the preprocessor, and is required at compile-time.
-* Document the X Macro and the meaning of its fields.
-* Error if the X Macro is not defined.
-* Include a sequence of X Macro uses, one per line, omitting following semicolons.
- Omitting semicolons allows X Macros to be used in either expression or statement position, which is useful.
-* Undefine the X Macro name at the end of the file.
-
-X Macro field values should be valid C constant literals.
-The first field of an X Macro should be the primary identifier, as shown in the example.
-
-X Macros should be kept as simple as possible.
-X Macro files should endeavour to only define one kind of X Macro, and separate files should be used for other X Macros.
-If possible, X Macros should only be used in implementation files, and should be avoided in headers.
-
-The code using an X Macro must:
-
-* Define the X Macro name, including any separators (such as semicolons or commas) within the definition.
- It is usually clearer if you split the definition over multiple lines.
-* Immediately after the definition, include the `.def` file for the X Macro.
-
-
+X macros that are not part of a header's API should be `#undef`ed after they are not needed.
+Similarly, the arguments to an X macro, if they are defined in a header, should be `#undef`ed too.
+This is not necessary in a `.c` or `.cc` file, where this cannot cause downstream namespace pollution.
## C++ Style Guide {#cxx-style-guide}
diff --git a/sw/device/lib/runtime/BUILD b/sw/device/lib/runtime/BUILD
index 635f41a..c0aad80 100644
--- a/sw/device/lib/runtime/BUILD
+++ b/sw/device/lib/runtime/BUILD
@@ -71,11 +71,11 @@
srcs = ["pmp.c"],
hdrs = ["pmp.h"],
target_compatible_with = [OPENTITAN_CPU],
- textual_hdrs = ["pmp_regions.def"],
deps = [
"//sw/device/lib/base:bitfield",
"//sw/device/lib/base:csr",
"//sw/device/lib/base:macros",
+ "//sw/device/lib/base/freestanding",
],
)
diff --git a/sw/device/lib/runtime/pmp.c b/sw/device/lib/runtime/pmp.c
index 2749f7c..e83afa0 100644
--- a/sw/device/lib/runtime/pmp.c
+++ b/sw/device/lib/runtime/pmp.c
@@ -43,6 +43,39 @@
};
/**
+ * This is an X-Macro used for automatically deriving switch statements which
+ * link PMP region identifiers to associated information including their
+ * configuration register identifier.
+ *
+ * This macro should be invoked with a macro argument with the following
+ * signature:
+ *
+ * @param region_id PMP Region Identifier.
+ * @param config_reg_id Configuration Register ID for a given PMP Region (for
+ * Ibex).
+ */
+#define PMP_REGIONS(X) \
+ X(0, 0) \
+ X(1, 0) \
+ X(2, 0) \
+ X(3, 0) \
+ \
+ X(4, 1) \
+ X(5, 1) \
+ X(6, 1) \
+ X(7, 1) \
+ \
+ X(8, 2) \
+ X(9, 2) \
+ X(10, 2) \
+ X(11, 2) \
+ \
+ X(12, 3) \
+ X(13, 3) \
+ X(14, 3) \
+ X(15, 3)
+
+/**
* Reads the pmpcfg for a given region.
*
* This reads the entire `pmpcfgN` value, not just the word associated with the
@@ -55,13 +88,14 @@
*/
OT_WARN_UNUSED_RESULT
static bool pmp_cfg_csr_read(pmp_region_index_t region, uint32_t *value) {
- switch (region) {
-#define PMP_REGION(region_id, config_reg_id) \
+#define PMP_READ_CONFIG_(region_id, config_reg_id) \
case region_id: { \
CSR_READ(CSR_REG_PMPCFG##config_reg_id, value); \
return true; \
}
-#include "sw/device/lib/runtime/pmp_regions.def"
+
+ switch (region) {
+ PMP_REGIONS(PMP_READ_CONFIG_)
default:
return false;
}
@@ -80,13 +114,14 @@
*/
OT_WARN_UNUSED_RESULT
static bool pmp_cfg_csr_write(pmp_region_index_t region, uint32_t value) {
- switch (region) {
-#define PMP_REGION(region_id, config_reg_id) \
+#define PMP_WRITE_CONFIG_(region_id, config_reg_id) \
case region_id: { \
CSR_WRITE(CSR_REG_PMPCFG##config_reg_id, value); \
return true; \
}
-#include "sw/device/lib/runtime/pmp_regions.def"
+
+ switch (region) {
+ PMP_REGIONS(PMP_WRITE_CONFIG_)
default:
return false;
}
@@ -102,13 +137,14 @@
*/
OT_WARN_UNUSED_RESULT
static bool pmp_addr_csr_read(pmp_region_index_t region, uint32_t *value) {
- switch (region) {
-#define PMP_REGION(region_id, _) \
+#define PMP_READ_ADDR_(region_id, _) \
case region_id: { \
CSR_READ(CSR_REG_PMPADDR##region_id, value); \
return true; \
}
-#include "sw/device/lib/runtime/pmp_regions.def"
+
+ switch (region) {
+ PMP_REGIONS(PMP_READ_ADDR_)
default:
return false;
}
@@ -123,13 +159,14 @@
*/
OT_WARN_UNUSED_RESULT
static bool pmp_addr_csr_write(pmp_region_index_t region, uint32_t value) {
- switch (region) {
-#define PMP_REGION(region_id, _) \
+#define PMP_WRITE_ADDR_(region_id, _) \
case region_id: { \
CSR_WRITE(CSR_REG_PMPADDR##region_id, value); \
return true; \
}
-#include "sw/device/lib/runtime/pmp_regions.def"
+
+ switch (region) {
+ PMP_REGIONS(PMP_WRITE_ADDR_)
default:
return false;
}
diff --git a/sw/device/lib/runtime/pmp_regions.def b/sw/device/lib/runtime/pmp_regions.def
deleted file mode 100644
index 5bf8680..0000000
--- a/sw/device/lib/runtime/pmp_regions.def
+++ /dev/null
@@ -1,49 +0,0 @@
-// Copyright lowRISC contributors.
-// Licensed under the Apache License, Version 2.0, see LICENSE for details.
-// SPDX-License-Identifier: Apache-2.0
-
-/**
- * \def PMP_REGION(region_id, config_reg_id)
- *
- * This is an X-Macro used for automatically deriving switch statements which
- * link PMP region identifiers to associated information including their
- * configuration register identifier.
- *
- * This also acts as the definitive list of every single PMP region that is
- * supported by `sw/device/lib/runtime/pmp.h`.
- *
- * This macro will be invoked without a following semicolon or comma.
- *
- * The macro will be undef'd at the end of this file, so this file may be
- * included multiple times in the same implementation file. Each inclusion site
- * should define `PMP_REGION` as required before including this file.
- *
- * @param region_id PMP Region Identifier.
- * @param config_reg_id Configuration Register ID for a given PMP Region (for
- * Ibex).
- */
-#ifndef PMP_REGION
-#error PMP_REGION(region_id, config_reg_id) must be defined
-#endif // PMP_REGION
-
-PMP_REGION(0, 0)
-PMP_REGION(1, 0)
-PMP_REGION(2, 0)
-PMP_REGION(3, 0)
-
-PMP_REGION(4, 1)
-PMP_REGION(5, 1)
-PMP_REGION(6, 1)
-PMP_REGION(7, 1)
-
-PMP_REGION(8, 2)
-PMP_REGION(9, 2)
-PMP_REGION(10, 2)
-PMP_REGION(11, 2)
-
-PMP_REGION(12, 3)
-PMP_REGION(13, 3)
-PMP_REGION(14, 3)
-PMP_REGION(15, 3)
-
-#undef PMP_REGION
