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

[Ashwin Poduval via Phabricator via cfe-commits]

ashwin98 added a comment.

In D117929#3314355 <https://reviews.llvm.org/D117929#3314355>, @ashwin98 wrote:

> 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 that is causing problems during the instrumentation map initialization.

I traced the root cause of the issue.  It seems to be stemming from the instrumentation map's relocation handling, specifically, at line 129 of InstrumentationMap.cpp, when we try to extract the load address of the symbol. I believe that section of the code was written keeping AArch64 in mind, but I'm not too sure about what changes will need to be made to add RISCV compatibility, I'm trying to figure it out.


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

https://reviews.llvm.org/D117929