This is an archive of the discontinued LLVM Phabricator instance.

[MC] Use `MCRegister` instead of `unsigned` in `MCTargetAsmParser`
ClosedPublic

Authored by barannikov88 on Dec 18 2022, 10:53 AM.

Diff Detail

Event Timeline

barannikov88 created this revision.Dec 18 2022, 10:53 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 18 2022, 10:53 AM
barannikov88 requested review of this revision.Dec 18 2022, 10:53 AM

Can you also fix ARC/AVR/M68k/LoongArch in this patch? -DLLVM_ERIMENTAL_TARGETS_TO_BUILD='ARC;AVR;LoongArch;M68k'

llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
4941

Update the RegNo parameter of parseSEHRegisterNumber

Can you also fix ARC/AVR/M68k/LoongArch in this patch? -DLLVM_ERIMENTAL_TARGETS_TO_BUILD='ARC;AVR;LoongArch;M68k'

Yes, sure.

Update experimental targets

@MaskRay
I've updated experimental targets.
Apparently, LoongArch has unmet dependencies in unittests/Target/LoongArch, which causes shared library build to fail.
Should I fix those in this PR? The fix is just to add three lines to CMakeLists.txt

MaskRay accepted this revision.Dec 18 2022, 11:39 AM

Looks great!

This revision is now accepted and ready to land.Dec 18 2022, 11:39 AM

lib/Target/*/AsmParser and tablegen generated files still have many unsigned RegNo or unsigned RegNum, but this is a step forward...

@MaskRay
I've updated experimental targets.
Apparently, LoongArch has unmet dependencies in unittests/Target/LoongArch, which causes shared library build to fail.
Should I fix those in this PR? The fix is just to add three lines to CMakeLists.txt

Perhaps fix that in a separate commit?

Perhaps fix that in a separate commit?

Fine by me.

lib/Target/*/AsmParser and tablegen generated files still have many unsigned RegNo or unsigned RegNum, but this is a step forward...

I hoped no one would remember it. Those are less annoying though, so maybe next time.
Can I ask you to merge this patch? I still don't have permission.

"Sergei Barannikov <barannikov88@gmail.com>".

barannikov88 added inline comments.Dec 18 2022, 12:00 PM
llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
4941

The parameter has MCRegister type. Did you mean something else?

Perhaps fix that in a separate commit?

Fine by me.

OK, I fixed it in 68c73bf7b236d4e6b9ac549c39a5fe3d701dfeee. Thanks for reporting the issue.

lib/Target/*/AsmParser and tablegen generated files still have many unsigned RegNo or unsigned RegNum, but this is a step forward...

I hoped no one would remember it. Those are less annoying though, so maybe next time.
Can I ask you to merge this patch? I still don't have permission.

"Sergei Barannikov <barannikov88@gmail.com>".

ninja -C /tmp/Rel check-llvm && llvm-push in one minute...

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

@MaskRay
Thank you for your time, I appreciate it.