This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Delete deprecated attributes
Needs RevisionPublic

Authored by zeng-xiao on Aug 22 2023, 7:25 PM.

Details

Summary

As described in riscv-elf-psabi, some attributes in risc-v have been deprecated.
The relevant link is: https://github.com/riscv-non-isa/riscv-elf-psabi-doc/commit/1b197a9f480ebc37835f8ef101a584e8a51fbe23

Diff Detail

Unit TestsFailed

Event Timeline

zeng-xiao created this revision.Aug 22 2023, 7:25 PM
Herald added a reviewer: MaskRay. · View Herald Transcript
Herald added a project: Restricted Project. · View Herald Transcript
zeng-xiao requested review of this revision.Aug 22 2023, 7:25 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 22 2023, 7:25 PM

[RISCV] Delete deprecated attributes

Update test case: riscv-attributes.s

MaskRay requested changes to this revision.EditedAug 23 2023, 12:29 AM

These are deprecated, not removed. lld needs to handle old object files with these attributes. The integrated assembler needs to support the syntax in case there are files using these directives.
I think we can only remove them when the compatibility with these object files/assembly files are deemed unnecessary. I am not sure when we claim to do that.

This revision now requires changes to proceed.Aug 23 2023, 12:29 AM

These are deprecated, not removed. lld needs to handle old object files with these attributes. The integrated assembler needs to support the syntax in case there are files using these directives.
I think we can only remove them when the compatibility with these object files/assembly files are deemed unnecessary. I am not sure when we claim to do that.

Okay, I understand what you mean. To be compatible with old elf files, these attributes in risc-v cannot be removed at this time. Perhaps one day in the future, we need to do this work.
This is my first llvm patch, and there will be other patches in the future.
Thank you, Professor Song's comment.
By the way, I am your fan. It's really surprising to have a conversation with you in this way.

zeng-xiao updated this revision to Diff 552655.Aug 23 2023, 4:30 AM

Updating D158575: [RISCV] Delete deprecated attributes

zeng-xiao updated this revision to Diff 552656.Aug 23 2023, 4:32 AM

Updating D158575: [RISCV] Delete deprecated attributes

MaskRay requested changes to this revision.Aug 23 2023, 11:08 AM

Updating D158575: [RISCV] Delete deprecated attributes

I think this patch should be abandoned. We need test coverage for the assembler and the linker. When I added the support to lld/ELF, I intentionally kept the deprecated cases

  // Attributes which use the default handling.
case RISCVAttrs::PRIV_SPEC:
case RISCVAttrs::PRIV_SPEC_MINOR:
case RISCVAttrs::PRIV_SPEC_REVISION:

to make it clear we need to support object files using them.

This revision now requires changes to proceed.Aug 23 2023, 11:08 AM

Updating D158575: [RISCV] Delete deprecated attributes

I think this patch should be abandoned. We need test coverage for the assembler and the linker. When I added the support to lld/ELF, I intentionally kept the deprecated cases

  // Attributes which use the default handling.
case RISCVAttrs::PRIV_SPEC:
case RISCVAttrs::PRIV_SPEC_MINOR:
case RISCVAttrs::PRIV_SPEC_REVISION:

to make it clear we need to support object files using them.

Okay, thank you. I will discard this patch.