This is an archive of the discontinued LLVM Phabricator instance.

[TableGen] Fix instantiating multiclass in foreach
ClosedPublic

Authored by jyyou.tw on Jan 24 2021, 2:16 AM.

Details

Summary

If multiclass argument comes from loop varaible and argument is
record type, it will not recognize the type.

This patch ensures that loop variables are resolved correctly.

Diff Detail

Event Timeline

jyyou.tw created this revision.Jan 24 2021, 2:16 AM
jyyou.tw requested review of this revision.Jan 24 2021, 2:16 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 24 2021, 2:16 AM

I presume this change passes all the TableGen, mlir, and clang tests?

llvm/lib/TableGen/TGParser.cpp
3528

Is there a reason why we shouldn't use !CurMultiClass?

llvm/test/TableGen/foreach-multiclass.td
123

Could you add a comment explaining what this is testing, since it is relatively obscure?

jyyou.tw updated this revision to Diff 319001.Jan 25 2021, 7:15 AM

Refine test case

jyyou.tw marked an inline comment as done.Jan 25 2021, 7:40 AM

I presume this change passes all the TableGen, mlir, and clang tests?

Yes, tests are passed.

llvm/lib/TableGen/TGParser.cpp
3528

If defm is placed in foreach and multiclass has foreach, the parsing flow is

parse foreach -> parse defm -> resolve multiclass entries -> resolve foreach of multiclass

In this case, !CurMultiClass is not enough. When resolving foreach of multiclass, Loop.ListValue is resolved as FieldInit only.
It must be further resolved, so I add Loops.empty() to ensure that.

llvm/lib/TableGen/TGParser.cpp
3528

Sorry, I didn't mean just !CurMultiClass. I meant:

!CurMultiClass && Loops.empty()

jyyou.tw updated this revision to Diff 319240.Jan 26 2021, 1:26 AM

Refine condition

jyyou.tw updated this revision to Diff 319243.Jan 26 2021, 1:33 AM
jyyou.tw marked an inline comment as done.

Missing test

Looks good to me.

This revision is now accepted and ready to land.Jan 26 2021, 6:58 AM

If this patch is OK, could someone please commit it? I don't have commit access.

Okay, let me do this one first. I've never committed for someone else, so something might go wrong.

This revision is pushed.