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

Fri Feb 11 08:21:45 PST 2022

ashwin98 added a comment.

In D117929#3299751 <https://reviews.llvm.org/D117929#3299751>, @dberris wrote:

> In D117929#3276843 <https://reviews.llvm.org/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 instrumentation to test compilation and linking, or add new ones for the llvm-xray tool under llvm/test/tools?
>
> Yes to both. :)
>
> There are already some tests that ensure the supported triples build with XRay instrumentation and that we can get the instrumentation map with the tooling on binaries generated with clang+lld. Extending those to support risc64 and ensure we're able to find the instrumentation with the tooling gives us higher confidence that this will work when the patch is accepted.

Before running the tests, I've been testing some of the subcommands with the generated logs. When xray account or stack was run directly on the log file, the data for the instrumented functions was output (I believe the XRay tool recognizes it as a YAML file and processes it as such). However, when passing the --instr_map flag, issues crop up. I originally ran into an error about the executable not being an ELF binary. I confirmed that clang was generating ELF binaries. There was a test checking the triple architecture in InstrumentationMap.cpp that seemed to be causing this.

On including RISCV64 to the check, I see new issues while reading the instrumentation map. How do I verify that the instrumentation map is being generated correctly (or if there is a problem with it), and where is the code that is responsible for the generation of the instrumentation map (is it in xray_init.cpp)? I'm not sure if this is a RISCV compatibility issue with the xray tool, or if I've missed something while initializing the instrumentation map.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D117929/new/

https://reviews.llvm.org/D117929