This is an archive of the discontinued LLVM Phabricator instance.

[NFC] Rename `FixedLenDecoderEmitter` as `DecoderEmitter`
ClosedPublic

Authored by 0x59616e on Apr 9 2022, 4:24 AM.

Details

Summary

Since now we are able to handle both fixed length & variable
length instructions.

Diff Detail

Event Timeline

0x59616e created this revision.Apr 9 2022, 4:24 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 9 2022, 4:24 AM
Herald added a subscriber: mgorny. · View Herald Transcript
0x59616e requested review of this revision.Apr 9 2022, 4:24 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 9 2022, 4:24 AM
0x59616e updated this revision to Diff 421717.Apr 9 2022, 4:38 AM

Rename the class name.

RKSimon added subscribers: myhsu, RKSimon.

No objections from me @myhsu ? Just a few minors to address.

There's some clang formatting that needs to be done since you've changed the length :-) of the class name

llvm/utils/TableGen/DecoderEmitter.cpp
2692–2698

clang format

llvm/utils/TableGen/DisassemblerEmitter.cpp
98–102

clang format

131

Raise a bug to track this?

142–145

clang format

149–151

clang format

0x59616e planned changes to this revision.EditedApr 9 2022, 7:46 AM

Thanks !

I've noticed the clang-format problem. The weird thing is, It happened after I rename this file, before I change anything in it. That makes me confusing. It should have not happened because I only change the file name.

So I want to take some time looking into it.

llvm/utils/TableGen/DisassemblerEmitter.cpp
131

I change this TODO just for reflecting the status of this decoder emitter to avoid confusing someone.

Resolving this means we need to deploy our varlen encoding infra onto webassembly backend. I don't know whether this is a good idea since this is a non-trivial work I guess.

0x59616e updated this revision to Diff 421735.Apr 9 2022, 8:12 AM

I know what is going on.

clang-format only modifies the diff part. Since now I rename the file,

the whole file will be checked and modified by clang-format.

craig.topper added inline comments.
llvm/utils/TableGen/DecoderEmitter.cpp
481

Please don't reformat lines that aren't affected by your change. Use clang-format-diff.py

myhsu accepted this revision.Apr 10 2022, 10:38 AM

LGTM, please resolve the formatting issues though

llvm/utils/TableGen/DisassemblerEmitter.cpp
131

I prefer creating a bug for this (and ping the wasm backend maintainer) but leave the comment unchanged

This revision is now accepted and ready to land.Apr 10 2022, 10:38 AM
0x59616e updated this revision to Diff 423311.Apr 17 2022, 1:30 PM
0x59616e marked an inline comment as done.

address format issue

0x59616e updated this revision to Diff 423313.Apr 17 2022, 2:08 PM

Revert the TODO suggested my myhsu

0x59616e updated this revision to Diff 423315.Apr 17 2022, 2:42 PM

continue addressing formating issues

This revision was landed with ongoing or failed builds.May 2 2022, 12:37 PM
This revision was automatically updated to reflect the committed changes.