This is an archive of the discontinued LLVM Phabricator instance.

[Object] Add some more LoongArch support
ClosedPublic

Authored by xen0n on Nov 15 2022, 2:27 AM.

Details

Summary

Add ELFObjectFileBase::getLoongArchFeatures, and return the proper ELF
relative reloc type for LoongArch.

Diff Detail

Event Timeline

xen0n created this revision.Nov 15 2022, 2:27 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 15 2022, 2:27 AM
xen0n requested review of this revision.Nov 15 2022, 2:27 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 15 2022, 2:27 AM
SixWeining added inline comments.Nov 16 2022, 7:29 PM
llvm/lib/Object/ELF.cpp
226–227

Seems this can be tested in llvm/unittests/Object/ELFObjectFileTest.cpp:

// ELF relative relocation type test.
TEST(ELFObjectFileTest, RelativeRelocationTypeTest) {
  EXPECT_EQ(ELF::R_CKCORE_RELATIVE, getELFRelativeRelocationType(ELF::EM_CSKY));
}

or in llvm/unittests/Object/ELFTest.cpp:

TEST(ELFTest, getELFRelativeRelocationType) {
  EXPECT_EQ(ELF::R_VE_RELATIVE, getELFRelativeRelocationType(EM_VE));
}
llvm/lib/Object/ELFObjectFile.cpp
344

Maybe similar tests could be added like RISCV in D42629.

xen0n updated this revision to Diff 478048.Nov 26 2022, 12:49 AM

rebase and add to unit test case

xen0n updated this revision to Diff 478051.Nov 26 2022, 5:04 AM

Remove llvm-objdump's now unnecessary --mattr arguments in LoongArch
MC tests' RUN lines, like what D42629 did and @SixWeining suggested.

SixWeining accepted this revision.Nov 26 2022, 5:27 AM

LGTM. Thanks.

This revision is now accepted and ready to land.Nov 26 2022, 5:27 AM
MaskRay accepted this revision.Nov 26 2022, 8:20 PM

LGTM.

llvm/lib/Object/ELFObjectFile.cpp
353

It's only two features so don't bother with [[fallthrough]] at present. this comment is unnecessary.

xen0n updated this revision to Diff 478079.Nov 27 2022, 3:18 AM

Rebase and shorten code to avoid explanatory comment on why duplicating one line of code.

xen0n marked 3 inline comments as done.Nov 27 2022, 3:23 AM
xen0n added inline comments.
llvm/lib/Object/ELFObjectFile.cpp
353

The comment was there explaining why code was duplicated instead of the shorter and arguably equally easy-to-understand approach. If it is removed but the duplicated AddFeature remained, then different people's coding style preferences could clash and I wanted to avoid such unnecessary debates.

Since you asked, I've thus removed the comment and chose to [[fallthrough]] instead. It's only one line after all, and I believe even programmers so used to functional style code must retain some control-flow deciphering skills to navigate in large C++ codebase in the first place...

xen0n marked an inline comment as done.Nov 27 2022, 3:24 AM
This revision was automatically updated to reflect the committed changes.