Skip to content

Conversation

@svs-quic
Copy link
Contributor

@svs-quic svs-quic commented Jan 31, 2025

The enum added in 1401703 does not work for custom vendor CSRs due to the presence of a "." in the <vendor>.<csr> naming scheme required by the toolchain convention.

Fix this by adding a new EnumName to the SysReg class which replaces the "." with and "_" before passing it to tablegen.

The enum added in #1401703 does not work for custom vendor CSRs due
to the presence of a "." in the <vendor>.<csr> naming scheme required
by the toolchain convention.

Fix this by adding a new EnumName to the SysReg class which replaces the
"." with and "_" before passing it to tablegen.
@svs-quic svs-quic requested a review from topperc January 31, 2025 05:38
@svs-quic svs-quic requested a review from lenary January 31, 2025 05:38
@llvmbot
Copy link
Member

llvmbot commented Jan 31, 2025

@llvm/pr-subscribers-backend-risc-v

Author: Sudharsan Veeravalli (svs-quic)

Changes

The enum added in 1401703 does not work for custom vendor CSRs due to the presence of a "." in the <vendor>.<csr> naming scheme required by the toolchain convention.

Fix this by adding a new EnumName to the SysReg class which replaces the "." with and "_" before passing it to tablegen.


Full diff: https://github.com/llvm/llvm-project/pull/125172.diff

1 Files Affected:

  • (modified) llvm/lib/Target/RISCV/RISCVSystemOperands.td (+4-1)
diff --git a/llvm/lib/Target/RISCV/RISCVSystemOperands.td b/llvm/lib/Target/RISCV/RISCVSystemOperands.td
index 4c86103db99d26..cabcb9eda06b19 100644
--- a/llvm/lib/Target/RISCV/RISCVSystemOperands.td
+++ b/llvm/lib/Target/RISCV/RISCVSystemOperands.td
@@ -19,6 +19,9 @@ include "llvm/TableGen/SearchableTable.td"
 
 class SysReg<string name, bits<12> op> {
   string Name = name;
+  // Custom vendor CSRs have a "<vendor>." prefix. Convert these to "<vendor>_"
+  // before passing it to the SysRegEncodings GenericEnum below.
+  string EnumName = !subst(".", "_", name);
   bits<12> Encoding = op;
   // FIXME: add these additional fields when needed.
   // Privilege Access: Read and Write = 0, 1, 2; Read-Only = 3.
@@ -50,7 +53,7 @@ def SysRegsList : GenericTable {
 
 def SysRegEncodings : GenericEnum {
   let FilterClass = "SysReg";
-  let NameField = "Name";
+  let NameField = "EnumName";
   let ValueField = "Encoding";
 }
 

@svs-quic
Copy link
Contributor Author

There are no tests for this right now since there aren't any vendor CSRs in the codebase. I have pushed this in preparation for upstreaming the Qualcomm Xqci custom CSRs.

Copy link
Collaborator

@topperc topperc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@svs-quic svs-quic merged commit 3b2f4f4 into llvm:main Jan 31, 2025
10 checks passed
@llvm-ci
Copy link
Collaborator

llvm-ci commented Jan 31, 2025

LLVM Buildbot has detected a new failure on builder flang-aarch64-dylib running on linaro-flang-aarch64-dylib while building llvm at step 5 "build-unified-tree".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/50/builds/9672

Here is the relevant piece of the build log for the reference
Step 5 (build-unified-tree) failure: build (failure)
...
627.624 [981/1/5818] Building CXX object tools/mlir/lib/CAPI/Dialect/CMakeFiles/obj.MLIRCAPISPIRV.dir/SPIRV.cpp.o
627.816 [980/1/5819] Building CXX object tools/mlir/lib/CAPI/Dialect/CMakeFiles/obj.MLIRCAPITensor.dir/Tensor.cpp.o
627.936 [979/1/5820] Building CXX object tools/mlir/lib/CAPI/Dialect/CMakeFiles/obj.MLIRCAPITransformDialect.dir/Transform.cpp.o
628.013 [978/1/5821] Building CXX object tools/mlir/lib/CAPI/Dialect/CMakeFiles/obj.MLIRCAPITransformDialectTransforms.dir/TransformInterpreter.cpp.o
628.132 [977/1/5822] Building CXX object tools/mlir/lib/CAPI/Dialect/CMakeFiles/obj.MLIRCAPIQuant.dir/Quant.cpp.o
628.247 [976/1/5823] Building CXX object tools/mlir/lib/CAPI/Dialect/CMakeFiles/obj.MLIRCAPIOpenMP.dir/OpenMP.cpp.o
628.316 [975/1/5824] Building CXX object tools/mlir/lib/CAPI/Dialect/CMakeFiles/obj.MLIRCAPIPDL.dir/PDL.cpp.o
628.503 [974/1/5825] Building CXX object tools/mlir/lib/CAPI/Dialect/CMakeFiles/obj.MLIRCAPIVector.dir/Vector.cpp.o
628.758 [973/1/5826] Building CXX object tools/mlir/test/lib/Pass/CMakeFiles/MLIRTestPass.dir/TestSPIRVCPURunnerPipeline.cpp.o
640.249 [972/1/5827] Building CXX object tools/mlir/test/lib/Pass/CMakeFiles/MLIRTestPass.dir/TestPassManager.cpp.o
FAILED: tools/mlir/test/lib/Pass/CMakeFiles/MLIRTestPass.dir/TestPassManager.cpp.o 
/usr/local/bin/c++ -DGTEST_HAS_RTTI=0 -DMLIR_INCLUDE_TESTS -D_DEBUG -D_GLIBCXX_ASSERTIONS -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/home/tcwg-buildbot/worker/flang-aarch64-dylib/build/tools/mlir/test/lib/Pass -I/home/tcwg-buildbot/worker/flang-aarch64-dylib/llvm-project/mlir/test/lib/Pass -I/home/tcwg-buildbot/worker/flang-aarch64-dylib/build/tools/mlir/include -I/home/tcwg-buildbot/worker/flang-aarch64-dylib/llvm-project/mlir/include -I/home/tcwg-buildbot/worker/flang-aarch64-dylib/build/include -I/home/tcwg-buildbot/worker/flang-aarch64-dylib/llvm-project/llvm/include -I/home/tcwg-buildbot/worker/flang-aarch64-dylib/llvm-project/mlir/test/lib/Pass/../Dialect/Test -I/home/tcwg-buildbot/worker/flang-aarch64-dylib/build/tools/mlir/test/lib/Pass/../Dialect/Test -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -ffunction-sections -fdata-sections -Wundef -Werror=mismatched-tags -O3 -DNDEBUG -std=c++17  -fno-exceptions -funwind-tables -fno-rtti -UNDEBUG -MD -MT tools/mlir/test/lib/Pass/CMakeFiles/MLIRTestPass.dir/TestPassManager.cpp.o -MF tools/mlir/test/lib/Pass/CMakeFiles/MLIRTestPass.dir/TestPassManager.cpp.o.d -o tools/mlir/test/lib/Pass/CMakeFiles/MLIRTestPass.dir/TestPassManager.cpp.o -c /home/tcwg-buildbot/worker/flang-aarch64-dylib/llvm-project/mlir/test/lib/Pass/TestPassManager.cpp
In file included from /home/tcwg-buildbot/worker/flang-aarch64-dylib/llvm-project/mlir/test/lib/Pass/TestPassManager.cpp:10:
/home/tcwg-buildbot/worker/flang-aarch64-dylib/llvm-project/mlir/test/lib/Pass/../Dialect/Test/TestOps.h:148:10: fatal error: 'TestOps.h.inc' file not found
  148 | #include "TestOps.h.inc"
      |          ^~~~~~~~~~~~~~~
1 error generated.
ninja: build stopped: subcommand failed.

@svs-quic svs-quic deleted the csrenum branch January 31, 2025 09:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants