[PATCH] D98610: [RISCV] Support clang -fpatchable-function-entry && GNU function attribute 'patchable_function_entry'

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG6ab892793185: [RISCV] Support clang -fpatchable-function-entry GNU function attribute… (authored by MaskRay). Repository: rG LLVM Github

[PATCH] D98610: [RISCV] Support clang -fpatchable-function-entry && GNU function attribute 'patchable_function_entry'

MaskRay added inline comments. Comment at: llvm/test/CodeGen/RISCV/patchable-function-entry.ll:24 +; RVC-NEXT: c.jr ra +; CHECK: .section __patchable_function_entries,"awo",@progbits,f1{{$}} +; 32: .p2align 2 jrtc27 wrote: > Is there a

[PATCH] D98610: [RISCV] Support clang -fpatchable-function-entry && GNU function attribute 'patchable_function_entry'

MaskRay updated this revision to Diff 330759. MaskRay marked 5 inline comments as done. MaskRay added a comment. Change check prefixes from 32/64 to RV32/RV64 Rebase and use `MCInst getNop` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98610/new/

[PATCH] D98610: [RISCV] Support clang -fpatchable-function-entry && GNU function attribute 'patchable_function_entry'

jrtc27 added inline comments. Comment at: llvm/lib/Target/RISCV/RISCVInstrInfo.cpp:53 +void RISCVInstrInfo::getNoop(MCInst ) const { + if (STI.getFeatureBits()[RISCV::FeatureStdExtC]) MaskRay wrote: > jrtc27 wrote: > > I will forever wonder why TII didn't

[PATCH] D98610: [RISCV] Support clang -fpatchable-function-entry && GNU function attribute 'patchable_function_entry'

MaskRay added inline comments. Comment at: llvm/lib/Target/RISCV/RISCVInstrInfo.cpp:53 +void RISCVInstrInfo::getNoop(MCInst ) const { + if (STI.getFeatureBits()[RISCV::FeatureStdExtC]) jrtc27 wrote: > I will forever wonder why TII didn't make it `MCInst

[PATCH] D98610: [RISCV] Support clang -fpatchable-function-entry && GNU function attribute 'patchable_function_entry'

MaskRay updated this revision to Diff 330744. MaskRay marked 8 inline comments as done. MaskRay added a comment. Mention supported targets in clang/include/clang/Basic/AttrDocs.td Use 4 llc RUN lines and --riscv-no-aliases Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D98610: [RISCV] Support clang -fpatchable-function-entry && GNU function attribute 'patchable_function_entry'

jrtc27 added inline comments. Comment at: llvm/test/CodeGen/RISCV/patchable-function-entry.ll:1-3 +;; Test the function attribute "patchable-function-entry". +; RUN: llc -mtriple=riscv32 < %s | FileCheck %s --check-prefixes=CHECK,32 +; RUN: llc -mtriple=riscv64 < %s | FileCheck

[PATCH] D98610: [RISCV] Support clang -fpatchable-function-entry && GNU function attribute 'patchable_function_entry'

MaskRay marked 2 inline comments as done. MaskRay added inline comments. Comment at: llvm/lib/Target/RISCV/RISCVMCInstLower.cpp:247 } + return false; } jrtc27 wrote: > true, surely? false. We want to call `EmitToStreamer(*OutStreamer, TmpInst);` If a

[PATCH] D98610: [RISCV] Support clang -fpatchable-function-entry && GNU function attribute 'patchable_function_entry'

jrtc27 added inline comments. Comment at: llvm/test/CodeGen/RISCV/patchable-function-entry.ll:1-3 +;; Test the function attribute "patchable-function-entry". +; RUN: llc -mtriple=riscv32 < %s | FileCheck %s --check-prefixes=CHECK,32 +; RUN: llc -mtriple=riscv64 < %s | FileCheck

[PATCH] D98610: [RISCV] Support clang -fpatchable-function-entry && GNU function attribute 'patchable_function_entry'

aaron.ballman added a comment. The attribute bits look fine to me aside from the documentation request. Comment at: clang/include/clang/Basic/Attr.td:738 let Args = [UnsignedArgument<"Count">, DefaultIntArgument<"Offset", 0>]; let Documentation =

[PATCH] D98610: [RISCV] Support clang -fpatchable-function-entry && GNU function attribute 'patchable_function_entry'

luismarques added inline comments. Comment at: llvm/test/CodeGen/RISCV/patchable-function-entry.ll:1-3 +;; Test the function attribute "patchable-function-entry". +; RUN: llc -mtriple=riscv32 < %s | FileCheck %s --check-prefixes=CHECK,32 +; RUN: llc -mtriple=riscv64 < %s |

[PATCH] D98610: [RISCV] Support clang -fpatchable-function-entry && GNU function attribute 'patchable_function_entry'

jrtc27 added inline comments. Comment at: llvm/lib/Target/RISCV/RISCVInstrInfo.cpp:53 +void RISCVInstrInfo::getNoop(MCInst ) const { + if (STI.getFeatureBits()[RISCV::FeatureStdExtC]) I will forever wonder why TII didn't make it `MCInst getNoop()`...

[PATCH] D98610: [RISCV] Support clang -fpatchable-function-entry && GNU function attribute 'patchable_function_entry'

MaskRay updated this revision to Diff 330548. MaskRay added a comment. fix comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98610/new/ https://reviews.llvm.org/D98610 Files: clang/include/clang/Basic/Attr.td

[PATCH] D98610: [RISCV] Support clang -fpatchable-function-entry && GNU function attribute 'patchable_function_entry'

luismarques accepted this revision. luismarques added a comment. This revision is now accepted and ready to land. Overall LGTM. I just don't understand what you mean with "1-byte NOPs" in the patchable prefix case. Regular NOPs are emitted. Please clarify the comment/patch as needed.

[PATCH] D98610: [RISCV] Support clang -fpatchable-function-entry && GNU function attribute 'patchable_function_entry'

MaskRay created this revision. MaskRay added reviewers: asb, craig.topper, HsiangKai, khchen. Herald added subscribers: vkmr, frasercrmck, evandro, luismarques, apazos, sameer.abuasal, pengfei, s.egerton, Jim, benna, psnobl, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01,