Page MenuHomePhabricator

[RISCV][MC] Delete unnecessary code section align
AcceptedPublic

Authored by StephenFan on Jun 11 2022, 6:59 AM.

Details

Summary

Since RISCVELFTargetObjectFile does not inherit RISCVMCObjectFileInfo,
the alignment of code section is always 4 even if c extension was enabled.
If c extension was enabled, the nop instruction is 2 bytes, which will
make the assember to emit a code align fixup.

Diff Detail

Unit TestsFailed

TimeTest
60,050 msx64 debian > MLIR.Examples/standalone::test.toy
Script: -- : 'RUN: at line 1'; /usr/bin/cmake /var/lib/buildkite-agent/builds/llvm-project/mlir/examples/standalone -G "Ninja" -DCMAKE_CXX_COMPILER=/usr/bin/clang++ -DCMAKE_C_COMPILER=/usr/bin/clang -DLLVM_ENABLE_LIBCXX=OFF -DMLIR_DIR=/var/lib/buildkite-agent/builds/llvm-project/build/lib/cmake/mlir -DLLVM_USE_LINKER=lld

Event Timeline

StephenFan created this revision.Jun 11 2022, 6:59 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 11 2022, 6:59 AM
StephenFan requested review of this revision.Jun 11 2022, 6:59 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 11 2022, 6:59 AM

Err, multiple virtual inheritance is tricky. Anyway to avoid it?

Could we make the text segment alignment a member variable of MCObjectInfo instead of needing to use a virtual method in MCObjectInfo? Then we'd just need to assign that member from RISCVMCObjectInfo and
RISCVTargetObjectFile.

  1. Add setTextSectionAlignment function to allow target set text section alignment
  2. Set RISCV's text section alignment when initialize RISCVELFTargetObjectFile
  3. Set RISCV's text section alignment when create RISCVMCObjectFileInfo
craig.topper added inline comments.Jun 28 2022, 3:20 PM
llvm/lib/Target/RISCV/RISCVSubtarget.cpp
22 ↗(On Diff #439275)

This is the only change in this file. Is it needed?

llvm/lib/Target/RISCV/RISCVTargetObjectFile.h
13 ↗(On Diff #439275)

There no changes to this file other than include, is it really needed here?

Remove header file.

This revision is now accepted and ready to land.Jun 28 2022, 8:48 PM
MaskRay accepted this revision.Jun 28 2022, 11:06 PM
MaskRay added inline comments.
llvm/test/CodeGen/RISCV/unnecessary-code-align.ll
5
; CHECK:      Relocations [
; CHECK-NEXT: ]

Remove CHECK-NOT.

If this tests alignment, you probably want to check llvm-readelf -S -r output. -S for the Al column. -r is to check there is no relocation.

Check llvm-readelf -S -r instead of llvm-readobj -r.