This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Fix state persistence bugs (PR55548)
ClosedPublic

Authored by kito-cheng on May 18 2022, 9:52 AM.

Details

Summary

We didn't implement RISCVELFStreamer::reset and cause some very strange
section output for attribute section...just reference D15950 to see how
ARM implement that.

Diff Detail

Event Timeline

kito-cheng created this revision.May 18 2022, 9:52 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 18 2022, 9:52 AM
kito-cheng requested review of this revision.May 18 2022, 9:52 AM
jrtc27 added inline comments.May 18 2022, 9:57 AM
llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp
234

What does this comment achieve?

llvm/test/MC/RISCV/twice.ll
22

Use -mtriple=riscv64 in the llc command and drop these two lines?

Changes:

kito-cheng marked 2 inline comments as done.May 19 2022, 7:49 PM
jrtc27 added inline comments.May 19 2022, 7:54 PM
llvm/test/MC/RISCV/twice.ll
7–8

Changes:

  • Refine testcase.
kito-cheng marked an inline comment as done.May 19 2022, 9:08 PM
MaskRay accepted this revision.May 20 2022, 12:03 PM
MaskRay added inline comments.
llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp
236

RTS should be omitted.

llvm/test/MC/RISCV/twice.ll
2

In some directories, the ## style is used to make comments stand out from RUN/CHECK lines. MC/RISCV. It will also make update_*checks.py's job easy.


I feel that CodeGen/RISCV may be more suitable but I can be persuaded that MC/RISCV is fine.

11
This revision is now accepted and ready to land.May 20 2022, 12:03 PM
kito-cheng marked 3 inline comments as done.May 26 2022, 1:01 AM
kito-cheng added inline comments.
llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp
236

MCTargetStreamer didn't provide reset, that's provided by RISCVTargetStreamer.

This revision was landed with ongoing or failed builds.May 26 2022, 1:09 AM
This revision was automatically updated to reflect the committed changes.
kito-cheng marked an inline comment as done.
MaskRay added inline comments.May 26 2022, 6:11 PM
llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp
236

My comment is more about: defining a variable is unnecessary.