[PATCH] D117929: [XRay] Add support for RISCV

ashwin98 added inline comments. Comment at: compiler-rt/lib/xray/xray_trampoline_riscv32.S:54 + // Handler address will be null if it is not set + beq a2, x0, FunctionEntry_restore + MaskRay wrote: > This local symbol doesn't seem useful. We can j

[PATCH] D117929: [XRay] Add support for RISCV

ashwin98 updated this revision to Diff 555925. ashwin98 marked an inline comment as done. ashwin98 added a comment. Sorry for the delay in getting back to you. I've updated the NOP sled generation and tested it out, it now seems to work with compressed instructions - the patched sleds were ident

[PATCH] D117929: [XRay] Add support for RISCV

MaskRay added inline comments. Comment at: compiler-rt/lib/xray/xray_trampoline_riscv32.S:54 + // Handler address will be null if it is not set + beq a2, x0, FunctionEntry_restore + This local symbol doesn't seem useful. We can just use numbers (`

[PATCH] D117929: [XRay] Add support for RISCV

ashwin98 updated this revision to Diff 554513. ashwin98 added a comment. Sorry - I missed adding updates with the last diff - I think I've addressed most of the comments. Just realized that the diff didn't include context, updated that as well. CHANGES SINCE LAST ACTION https://reviews.llvm.

[PATCH] D117929: [XRay] Add support for RISCV

ashwin98 added inline comments. Comment at: compiler-rt/cmake/Modules/AllSupportedArchDefs.cmake:83 set(ALL_XRAY_SUPPORTED_ARCH ${X86_64} ${ARM32} ${ARM64} ${MIPS32} ${MIPS64} - powerpc64le ${HEXAGON} ${LOONGARCH64}) + powerpc64le ${HEXAGON} ${LOONGAR

[PATCH] D117929: [XRay] Add support for RISCV

ashwin98 updated this revision to Diff 553135. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117929/new/ https://reviews.llvm.org/D117929 Files: clang/lib/Driver/XRayArgs.cpp compiler-rt/cmake/Modules/AllSupportedArchDefs.cmake compiler-rt/lib/xray/CMakeLists.txt compiler-rt/lib/

[PATCH] D117929: [XRay] Add support for RISCV

MaskRay added a comment. > Context not available See https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface Comment at: compiler-rt/cmake/Modules/AllSupportedArchDefs.cmake:83 set(ALL_XRAY_SUPPORTED_ARCH ${X86_64} ${ARM32} ${ARM64} ${MIPS32} ${MIPS6

[PATCH] D117929: [XRay] Add support for RISCV

ashwin98 updated this revision to Diff 552054. ashwin98 added a reviewer: MaskRay. ashwin98 added a comment. Herald added a subscriber: sunshaoce. Ok, I've tried to make the changes you've suggested @MaskRay , let me know if I've missed something. On another note, I lost my previous qemu VM and

[PATCH] D117929: [XRay] Add support for RISCV

ashwin98 added a comment. In D117929#4445858 , @MaskRay wrote: > I am still interested in a RISC-V XRay port :) Sorry - lost track of this, things have been hectic for the last few months, but we also seem to have got custom events working with riscv64.

[PATCH] D117929: [XRay] Add support for RISCV

MaskRay added inline comments. Comment at: compiler-rt/lib/xray/xray_trampoline_riscv32.S:16 + .text + .file "xray_trampoline_riscv32.S" + .globl __xray_FunctionEntry Omit `.file` Consider using `#include "../sanitizer_common/sanitizer_asm.h"`

[PATCH] D117929: [XRay] Add support for RISCV

MaskRay added a comment. Herald added subscribers: wangpc, luke, Enna1, shiva0217, arichardson. Herald added a project: All. I am still interested in a RISC-V XRay port :) Comment at: llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp:280 + // Emit NOP instructions + for (int8_t I = 0

[PATCH] D117929: [XRay] Add support for RISCV

dberris added a comment. In D117929#3317813 , @ashwin98 wrote: > In D117929#3314355 , @ashwin98 > wrote: > >> On including RISCV64 to the check, I see new issues while reading the >> instrumentation map. How do

[PATCH] D117929: [XRay] Add support for RISCV

ashwin98 added a comment. In D117929#3314355 , @ashwin98 wrote: > In D117929#3299751 , @dberris wrote: > >> In D117929#3276843 , @ashwin98 >> wrote: >> >>> Fixed another

[PATCH] D117929: [XRay] Add support for RISCV

ashwin98 added a comment. In D117929#3299751 , @dberris wrote: > In D117929#3276843 , @ashwin98 > wrote: > >> Fixed another lint issue, they should all be done for now hopefully. >> >> @dberris Sorry, I'm a littl

[PATCH] D117929: [XRay] Add support for RISCV

dberris added a comment. In D117929#3276843 , @ashwin98 wrote: > Fixed another lint issue, they should all be done for now hopefully. > > @dberris Sorry, I'm a little confused, do you mean I need to update some of > the clang tests for XRay instrumentati

[PATCH] D117929: [XRay] Add support for RISCV

ashwin98 updated this revision to Diff 405621. ashwin98 added a comment. Removed extra triples from the test. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117929/new/ https://reviews.llvm.org/D117929 Files: clang/lib/Driver/XRayArgs.cpp compiler-rt/cmake/Modules/AllSupportedArchDe

[PATCH] D117929: [XRay] Add support for RISCV

ashwin98 added inline comments. Comment at: llvm/test/CodeGen/RISCV/xray-attribute-instrumentation.ll:1-2 +; RUN: llc -mtriple=riscv32-unknown-elf -mattr=+d -verify-machineinstrs < %s | FileCheck --check-prefix=CHECK %s +; RUN: llc -mtriple=riscv32-unknown-linux-gnu -mattr=+d -v

[PATCH] D117929: [XRay] Add support for RISCV

ashwin98 updated this revision to Diff 405432. ashwin98 added a comment. Updated the riscv64 sled function to get rid of the to fix the addition/shift operations and get rid of the superfluous slli and srli instructions. Cut out unnecessary comments in the ASM Printer. CHANGES SINCE LAST ACTIO

[PATCH] D117929: [XRay] Add support for RISCV

ashwin98 added inline comments. Comment at: compiler-rt/lib/xray/xray_riscv.cpp:122-123 + //lui t2, %highest(__xray_FunctionEntry/Exit) + //slli t2, t2, 32 ;lui sign extends values + //srli t2, t2, 32

[PATCH] D117929: [XRay] Add support for RISCV

jrtc27 added inline comments. Comment at: compiler-rt/lib/xray/xray_riscv.cpp:122-123 + //lui t2, %highest(__xray_FunctionEntry/Exit) + //slli t2, t2, 32 ;lui sign extends values + //srli t2, t2, 32 ;

[PATCH] D117929: [XRay] Add support for RISCV

ashwin98 updated this revision to Diff 403707. ashwin98 added a comment. Fixed another lint issue, they should all be done for now hopefully. @dberris Sorry, I'm a little confused, do you mean I need to update some of the clang tests for XRay instrumentation to test compilation and linking, or a

[PATCH] D117929: [XRay] Add support for RISCV

ashwin98 updated this revision to Diff 403331. ashwin98 added a comment. Made changes to handle lint issues. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117929/new/ https://reviews.llvm.org/D117929 Files: clang/lib/Driver/XRayArgs.cpp compiler-rt/cmake/Modules/AllSupportedArchDef

[PATCH] D117929: [XRay] Add support for RISCV

dberris requested changes to this revision. dberris added a comment. This revision now requires changes to proceed. It looks like you'll need to address the lint issues (using clang-format). I'm not an expert on RISCV assembly so you might need to get someone familiar with the ISA reviewing this

[PATCH] D117929: [XRay] Add support for RISCV

ashwin98 updated this revision to Diff 402533. ashwin98 added a comment. Herald added a subscriber: pcwang-thead. Updated the diff, made the following changes: 1. Merged the riscv files into xray_riscv.cpp and removed the if-else code for %hi() 2. Cleaned up the issues related to indenting and c

[PATCH] D117929: [XRay] Add support for RISCV

jrtc27 added a comment. In D117929#3263462 , @ashwin98 wrote: > Thank you for your feedback! I could combine the riscv32 and 64 cpp files > with some xlen conditions if that will work better, but that might take a bit > of a hit in terms of readability

[PATCH] D117929: [XRay] Add support for RISCV

ashwin98 added a comment. Thank you for your feedback! I could combine the riscv32 and 64 cpp files with some xlen conditions if that will work better, but that might take a bit of a hit in terms of readability (do I explain both sleds in the comments preceding the implementation). I have comme

[PATCH] D117929: [XRay] Add support for RISCV

jrtc27 added a comment. 1. Please upload patches with full context 2. You should not need to have separate xray_riscv32/64.cpp, a single shared file with a small amount of XLEN-based conditions should suffice and reduce a whole load of code duplication. Possibly also applies to the trampoline a

[PATCH] D117929: [XRay] Add support for RISCV

ashwin98 created this revision. ashwin98 added reviewers: dberris, asb. Herald added subscribers: VincentWu, luke957, achieveartificialintelligence, vkmr, frasercrmck, evandro, luismarques, apazos, sameer.abuasal, s.egerton, Jim, benna, psnobl, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rog