Page MenuHomePhabricator

[MC][RISCV] Add RISCV MCObjectFileInfo
Needs ReviewPublic

Authored by flip1995 on May 7 2021, 12:58 AM.

Details

Summary

This makes sure, that the text section will have a 2-byte alignment, if
the +c extension is enabled.

Diff Detail

Event Timeline

flip1995 created this revision.May 7 2021, 12:58 AM

The banners were copied from other files but not updated to match the new file.

IIRC no one was opposed to this alignment change when we discussed it in a recent RISC-V sync-up call, but let's give it some time for more people to chime in.

llvm/test/MC/RISCV/align.s
41–44

enable -> enabled

flip1995 updated this revision to Diff 347640.May 25 2021, 5:03 AM
flip1995 marked an inline comment as done.

rebased

flip1995 published this revision for review.May 25 2021, 5:05 AM
flip1995 added reviewers: MaskRay, asb.
Herald added a project: Restricted Project. · View Herald TranscriptMay 25 2021, 5:05 AM
MaskRay added inline comments.May 25 2021, 10:40 AM
llvm/lib/MC/MCObjectFileInfo.cpp
965

{}

= default doesn't change the fact that it is user-defined. IMHO {} is the prevailing style.

llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCObjectFileInfo.cpp
22

Use just one variable. Don't keep Ctx

24

Drop braces around simple statements.

Just use ? :

llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCObjectFileInfo.h
20

Is this used?

flip1995 updated this revision to Diff 348929.Jun 1 2021, 3:58 AM

addressed review comments and rebased

flip1995 updated this revision to Diff 348930.Jun 1 2021, 4:01 AM

Fixed clang-tidy error

riscv64-linux-gnu-as -march=rv64gc a.s does use 2-byte alignment, so I think this may be fine.

I just cannot find sufficient justification for such a change.

I just cannot find sufficient justification for such a change.

So besides getting the same behavior between GCC and LLVM, this change also improves code size for RISC-V. This is because there doesn't have to be padding added between functions, if they aren't 4 byte aligned.