Skip to content

Conversation

@mdehling
Copy link
Contributor

Fixes a crash (assertion failure) in mlir-tblgen -emit-enum-doc caused by calling EnumAttr() for the wrong type of Record *: EnumAttr rather than EnumAttrInfo as asserted.

Compare the corresponding line in emitDialectDoc():

auto enumDefs = records.getAllDerivedDefinitionsIfDefined("EnumAttrInfo");

Fixes a crash (assertion failure) in `mlir-tblgen -emit-enum-doc` caused
by calling `EnumAttr()` for the wrong type of `Record *`: `EnumAttr`
rather than `EnumAttrInfo` as asserted.
@github-actions
Copy link

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

@llvmbot llvmbot added mlir:core MLIR Core Infrastructure mlir labels Nov 29, 2024
@llvmbot
Copy link
Member

llvmbot commented Nov 29, 2024

@llvm/pr-subscribers-mlir-core

@llvm/pr-subscribers-mlir

Author: Malte Dehling (mdehling)

Changes

Fixes a crash (assertion failure) in mlir-tblgen -emit-enum-doc caused by calling EnumAttr() for the wrong type of Record *: EnumAttr rather than EnumAttrInfo as asserted.

Compare the corresponding line in emitDialectDoc():

auto enumDefs = records.getAllDerivedDefinitionsIfDefined("EnumAttrInfo");


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

1 Files Affected:

  • (modified) mlir/tools/mlir-tblgen/OpDocGen.cpp (+1-1)
diff --git a/mlir/tools/mlir-tblgen/OpDocGen.cpp b/mlir/tools/mlir-tblgen/OpDocGen.cpp
index d499c78a5cf44d..1c394f5680a5c9 100644
--- a/mlir/tools/mlir-tblgen/OpDocGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpDocGen.cpp
@@ -409,7 +409,7 @@ static void emitEnumDoc(const EnumAttr &def, raw_ostream &os) {
 
 static void emitEnumDoc(const RecordKeeper &records, raw_ostream &os) {
   os << "<!-- Autogenerated by mlir-tblgen; don't manually edit -->\n";
-  for (const Record *def : records.getAllDerivedDefinitions("EnumAttr"))
+  for (const Record *def : records.getAllDerivedDefinitions("EnumAttrInfo"))
     emitEnumDoc(EnumAttr(def), os);
 }
 

@mdehling
Copy link
Contributor Author

Pinging @nikic @antiagainst @kuhar since you were reviewers for another recent mlir-tblgen pr. Could one of you have a quick look at this?

Copy link
Member

@kuhar kuhar left a comment

Choose a reason for hiding this comment

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

How can we reproduce this crash?

@mdehling
Copy link
Contributor Author

Thank you for taking the time.

To reproduce, run for example:

$ mlir-tblgen -gen-enum-doc mlir/include/mlir/Dialect/Linalg/IR/LinalgBase.td -I mlir/include
mlir-tblgen: /home/m.dehling/llvm-project/mlir/lib/TableGen/Attribute.cpp:169: mlir::tblgen::EnumAttr::EnumAttr(const llvm::Record *): Assertion `isSubClassOf("EnumAttrInfo") && "must be subclass of TableGen 'EnumAttr' class"' failed.
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.
Stack dump:
0.      Program arguments: ./build/bin/mlir-tblgen -gen-enum-doc mlir/include/mlir/Dialect/Linalg/IR/LinalgBase.td -I mlir/include
Stack dump without symbol names (ensure you have llvm-symbolizer in your PATH or set the environment var `LLVM_SYMBOLIZER_PATH` to point to it):
0  mlir-tblgen 0x000055f9b588a907
1  mlir-tblgen 0x000055f9b5888b40
2  mlir-tblgen 0x000055f9b588af9a
3  libc.so.6   0x00007fbc60c66520
4  libc.so.6   0x00007fbc60cba9fc pthread_kill + 300
5  libc.so.6   0x00007fbc60c66476 raise + 22
6  libc.so.6   0x00007fbc60c4c7f3 abort + 211
7  libc.so.6   0x00007fbc60c4c71b
8  libc.so.6   0x00007fbc60c5de96
9  mlir-tblgen 0x000055f9b58e44f5
10 mlir-tblgen 0x000055f9b57e993c
11 mlir-tblgen 0x000055f9b58c2d76
12 mlir-tblgen 0x000055f9b58c5836
13 mlir-tblgen 0x000055f9b58c258f
14 libc.so.6   0x00007fbc60c4dd90
15 libc.so.6   0x00007fbc60c4de40 __libc_start_main + 128
16 mlir-tblgen 0x000055f9b5744355
Aborted (core dumped)

You will notice the assertion error doesn't match the condition checked.

Copy link
Member

@kuhar kuhar left a comment

Choose a reason for hiding this comment

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

LGTM.

Thanks for the repro, it does fix the crash.

@kuhar kuhar merged commit e5521fa into llvm:main Dec 17, 2024
11 checks passed
@github-actions
Copy link

@mdehling Congratulations on having your first Pull Request (PR) merged into the LLVM Project!

Your changes will be combined with recent changes from other authors, then tested by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR.

Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues.

How to do this, and the rest of the post-merge process, is covered in detail here.

If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again.

If you don't get any reports, no action is required from you. Your changes are working as expected, well done!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

mlir:core MLIR Core Infrastructure mlir

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants