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

[Ashwin Poduval via Phabricator via cfe-commits]

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 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/xray/xray_interface.cpp
  compiler-rt/lib/xray/xray_riscv.cpp
  compiler-rt/lib/xray/xray_trampoline_riscv32.S
  compiler-rt/lib/xray/xray_trampoline_riscv64.S
  compiler-rt/lib/xray/xray_tsc.h
  llvm/lib/CodeGen/XRayInstrumentation.cpp
  llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
  llvm/lib/Target/RISCV/RISCVSubtarget.h
  llvm/test/CodeGen/RISCV/xray-attribute-instrumentation.ll

Index: llvm/test/CodeGen/RISCV/xray-attribute-instrumentation.ll
===
--- /dev/null
+++ llvm/test/CodeGen/RISCV/xray-attribute-instrumentation.ll
@@ -0,0 +1,58 @@
+; 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 -verify-machineinstrs < %s | FileCheck --check-prefix=CHECK %s
+; RUN: llc -mtriple=riscv64-unknown-elf -mattr=+d -verify-machineinstrs < %s | FileCheck --check-prefix=CHECK --check-prefix=CHECK-RISCV64 %s
+; RUN: llc -mtriple=riscv64-unknown-linux-gnu -mattr=+d -verify-machineinstrs < %s | FileCheck --check-prefix=CHECK --check-prefix=CHECK-RISCV64 %s
+
+define i32 @foo() nounwind "function-instrument"="xray-always" {
+; CHECK:.p2align 2
+; CHECK-LABEL:  .Lxray_sled_0:
+; CHECK-NEXT:   j .Ltmp0
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-LABEL:  .Ltmp0:
+  ret i32 0
+; CHECK:.p2align 2
+; CHECK-LABEL:  .Lxray_sled_1:
+; CHECK-NEXT:   j .Ltmp1
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-LABEL:  .Ltmp1:
+; CHECK-NEXT:   ret
+}
+; CHECK:.section xray_instr_map,"ao",@progbits,foo
+; CHECK-LABEL:  .Lxray_sleds_start0:
+; CHECK:.Lxray_sled_0-.Ltmp2
+; CHECK:.Lxray_sled_1-.Ltmp3
+; CHECK-LABEL:  .Lxray_sleds_end0:
Index: llvm/lib/Target/RISCV/RISCVSubtarget.h
===
--- llvm/lib/Target/RISCV/RISCVSubtarget.h
+++ llvm/lib/Target/RISCV/RISCVSubtarget.h
@@ -187,6 +187,11 @@
   unsigned getMaxInterleaveFactor() const {
 return hasVInstructions() ? MaxInterleaveFactor : 1;
   }
+  // Add XRay support - needs double precision floats at present and does not
+  // support compressed instructions
+  bool isXRaySupported() const override {
+return hasStdExtD() && !hasStdExtC();
+  }
 
 protected:
   // GlobalISel related APIs.
Index: llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
===
--- llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
+++ llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
@@ -15,6 +15,7 @@
 #include "MCTargetDesc/RISCVMCExpr.h"
 #include "MCTargetDesc/RISCVTargetStreamer.h"
 #include "RISCV.h"
+#include "RISCVSubtarget.h"
 #include "RISCVTargetMachine.h"
 #include "TargetInfo/RISCVTargetInfo.h"
 #include "llvm/ADT/Statistic.h"
@@ -25,10 +26,12 @@
 #include "llvm/CodeGen/MachineModuleInfo.h"
 #include "llvm/MC/MCAsmInfo.h"
 #include "llvm/MC/MCInst.h"
+#include "llvm/MC/MCInstBuilder.h"
 #include "llvm/MC/MCStreamer.h"
 #include "llvm/MC/MCSymbol.h"
 #include "llvm/MC/TargetRegistry.h"
 #include "llvm/Support/raw_ostream.h"
+#include 
 using namespace llvm;
 
 #define DEBUG_TYPE "asm-printer"
@@ -65,11 +68,18 @@
 return LowerRISCVMachineOperandToMCOperand(MO, MCOp, *this);
   }
 
+  // XRay Support
+  void LowerPATCHABLE_FUNCTION_ENTER(const MachineInstr *MI);
+  void LowerPATCHABLE_FUNCTION_EXIT(const Mach

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

[Ashwin Poduval via Phabricator via cfe-commits]

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 -verify-machineinstrs 
< %s | FileCheck --check-prefix=CHECK %s
+; RUN: llc -mtriple=riscv64-unknown-elf -mattr=+d -verify-machineinstrs < %s | 
FileCheck --check-prefix=CHECK --check-prefix=CHECK-RISCV64 %s

jrtc27 wrote:
> Triples are overly verbose; riscv32-unknown-elf is normally just written 
> riscv32, and riscv32-unknown-linux-gnu as riscv64-linux-gnu, though I don't 
> see what point having both serves, we normally only use the bare-metal 
> triples unless something has an OS-specific aspect
I will cut it down to just the ones with linux. Clang has a check to see if the 
OS is Linux when passed the flag for XRay instrumentation.


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

https://reviews.llvm.org/D117929

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Ashwin Poduval via Phabricator via cfe-commits]

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/AllSupportedArchDefs.cmake
  compiler-rt/lib/xray/CMakeLists.txt
  compiler-rt/lib/xray/xray_interface.cpp
  compiler-rt/lib/xray/xray_riscv.cpp
  compiler-rt/lib/xray/xray_trampoline_riscv32.S
  compiler-rt/lib/xray/xray_trampoline_riscv64.S
  compiler-rt/lib/xray/xray_tsc.h
  llvm/lib/CodeGen/XRayInstrumentation.cpp
  llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
  llvm/lib/Target/RISCV/RISCVSubtarget.h
  llvm/test/CodeGen/RISCV/xray-attribute-instrumentation.ll

Index: llvm/test/CodeGen/RISCV/xray-attribute-instrumentation.ll
===
--- /dev/null
+++ llvm/test/CodeGen/RISCV/xray-attribute-instrumentation.ll
@@ -0,0 +1,56 @@
+; RUN: llc -mtriple=riscv32-unknown-linux-gnu -mattr=+d -verify-machineinstrs < %s | FileCheck --check-prefix=CHECK %s
+; RUN: llc -mtriple=riscv64-unknown-linux-gnu -mattr=+d -verify-machineinstrs < %s | FileCheck --check-prefix=CHECK --check-prefix=CHECK-RISCV64 %s
+
+define i32 @foo() nounwind "function-instrument"="xray-always" {
+; CHECK:.p2align 2
+; CHECK-LABEL:  .Lxray_sled_0:
+; CHECK-NEXT:   j .Ltmp0
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-LABEL:  .Ltmp0:
+  ret i32 0
+; CHECK:.p2align 2
+; CHECK-LABEL:  .Lxray_sled_1:
+; CHECK-NEXT:   j .Ltmp1
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-LABEL:  .Ltmp1:
+; CHECK-NEXT:   ret
+}
+; CHECK:.section xray_instr_map,"ao",@progbits,foo
+; CHECK-LABEL:  .Lxray_sleds_start0:
+; CHECK:.Lxray_sled_0-.Ltmp2
+; CHECK:.Lxray_sled_1-.Ltmp3
+; CHECK-LABEL:  .Lxray_sleds_end0:
Index: llvm/lib/Target/RISCV/RISCVSubtarget.h
===
--- llvm/lib/Target/RISCV/RISCVSubtarget.h
+++ llvm/lib/Target/RISCV/RISCVSubtarget.h
@@ -187,6 +187,11 @@
   unsigned getMaxInterleaveFactor() const {
 return hasVInstructions() ? MaxInterleaveFactor : 1;
   }
+  // Add XRay support - needs double precision floats at present and does not
+  // support compressed instructions
+  bool isXRaySupported() const override {
+return hasStdExtD() && !hasStdExtC();
+  }
 
 protected:
   // GlobalISel related APIs.
Index: llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
===
--- llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
+++ llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
@@ -15,6 +15,7 @@
 #include "MCTargetDesc/RISCVMCExpr.h"
 #include "MCTargetDesc/RISCVTargetStreamer.h"
 #include "RISCV.h"
+#include "RISCVSubtarget.h"
 #include "RISCVTargetMachine.h"
 #include "TargetInfo/RISCVTargetInfo.h"
 #include "llvm/ADT/Statistic.h"
@@ -25,10 +26,12 @@
 #include "llvm/CodeGen/MachineModuleInfo.h"
 #include "llvm/MC/MCAsmInfo.h"
 #include "llvm/MC/MCInst.h"
+#include "llvm/MC/MCInstBuilder.h"
 #include "llvm/MC/MCStreamer.h"
 #include "llvm/MC/MCSymbol.h"
 #include "llvm/MC/TargetRegistry.h"
 #include "llvm/Support/raw_ostream.h"
+#include 
 using namespace llvm;
 
 #define DEBUG_TYPE "asm-printer"
@@ -65,11 +68,18 @@
 return LowerRISCVMachineOperandToMCOperand(MO, MCOp, *this);
   }
 
+  // XRay Support
+  void LowerPATCHABLE_FUNCTION_ENTER(const MachineInstr *MI);
+  void LowerPATCHABLE_FUNCTION_EXIT(const MachineInstr *MI);
+  void LowerPATCHABLE_TAIL_CALL(const MachineInstr *MI);
+
   void emitStartOfAsmFile(Module &M) override;
   void emitEndOfAsmFile(Module &M) override;
 
 private:
   void emitAttributes();
+  // XRay Support
+  void emitSled(const MachineInstr *MI, SledKind Kind);
 };
 }
 
@@ -92,6 +102,30 @@
   if (emitPseudoExpansionLowering(*OutStreamer, MI))
 return;
 
+  switch (MI->getOpcode()) {
+  case 

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

[Ashwin Poduval via Phabricator via cfe-commits]

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 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

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Ashwin Poduval via Phabricator via cfe-commits]

ashwin98 added a comment.

In D117929#3314355 , @ashwin98 wrote:

> 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 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

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Ashwin Poduval via Phabricator via cfe-commits]

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 have rebuilt LLVM with this 
patch in a new one; I am now getting the following issue with the XRay runtime:

SanitizerTool: CHECK failed: sanitizer_allocator_primary32.h:292 "((res)) < 
((kNumPossibleRegions))" (0x7fff93a, 0x4) (tid=24873)

A previous thread (https://reviews.llvm.org/D87580) suggests this might be a 
qemu issue, but I am using qemu-system-riscv64 (and not qemu user-mode.) 
However, I figured I'd check anyway.


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/xray/xray_interface.cpp
  compiler-rt/lib/xray/xray_riscv.cpp
  compiler-rt/lib/xray/xray_trampoline_riscv32.S
  compiler-rt/lib/xray/xray_trampoline_riscv64.S
  compiler-rt/lib/xray/xray_tsc.h
  llvm/lib/CodeGen/XRayInstrumentation.cpp
  llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
  llvm/lib/Target/RISCV/RISCVSubtarget.h
  llvm/test/CodeGen/RISCV/xray-attribute-instrumentation.ll

Index: llvm/test/CodeGen/RISCV/xray-attribute-instrumentation.ll
===
--- /dev/null
+++ llvm/test/CodeGen/RISCV/xray-attribute-instrumentation.ll
@@ -0,0 +1,24 @@
+; RUN: llc -mtriple=riscv32-unknown-linux-gnu -mattr=+d -verify-machineinstrs < %s | FileCheck --check-prefix=CHECK %s
+; RUN: llc -mtriple=riscv64-unknown-linux-gnu -mattr=+d -verify-machineinstrs < %s | FileCheck --check-prefix=CHECK --check-prefix=CHECK-RISCV64 %s
+
+define i32 @foo() nounwind "function-instrument"="xray-always" {
+; CHECK: .p2align 2
+; CHECK-LABEL:   .Lxray_sled_0:
+; CHECK-NEXT:j .Ltmp0
+; CHECK-COUNT-14:nop
+; CHECK-RISCV64-COUNT-4: nop
+; CHECK-LABEL:   .Ltmp0:
+  ret i32 0
+; CHECK: .p2align 2
+; CHECK-LABEL:   .Lxray_sled_1:
+; CHECK-NEXT:j .Ltmp1
+; CHECK-COUNT-14:nop
+; CHECK-RISCV64-COUNT-4: nop
+; CHECK-LABEL:   .Ltmp1:
+; CHECK-NEXT:ret
+}
+; CHECK: .section xray_instr_map,"ao",@progbits,foo
+; CHECK-LABEL:   .Lxray_sleds_start0:
+; CHECK: .Lxray_sled_0-[[TMP:.Ltmp[0-9]+]]
+; CHECK: .Lxray_sled_1-[[TMP:.Ltmp[0-9]+]]
+; CHECK-LABEL:   .Lxray_sleds_end0:
Index: llvm/lib/Target/RISCV/RISCVSubtarget.h
===
--- llvm/lib/Target/RISCV/RISCVSubtarget.h
+++ llvm/lib/Target/RISCV/RISCVSubtarget.h
@@ -160,6 +160,11 @@
 assert(i < RISCV::NUM_TARGET_REGS && "Register out of range");
 return UserReservedRegister[i];
   }
+  // Add XRay support - needs double precision floats at present and does not
+  // support compressed instructions
+  bool isXRaySupported() const override {
+return hasStdExtD() && !hasStdExtC();
+  }
 
   bool hasMacroFusion() const { return hasLUIADDIFusion(); }
 
Index: llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
===
--- llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
+++ llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
@@ -16,6 +16,7 @@
 #include "MCTargetDesc/RISCVMCExpr.h"
 #include "MCTargetDesc/RISCVTargetStreamer.h"
 #include "RISCV.h"
+#include "RISCVSubtarget.h"
 #include "RISCVMachineFunctionInfo.h"
 #include "RISCVTargetMachine.h"
 #include "TargetInfo/RISCVTargetInfo.h"
@@ -84,6 +85,11 @@
   // Wrapper needed for tblgenned pseudo lowering.
   bool lowerOperand(const MachineOperand &MO, MCOperand &MCOp) const;
 
+  // XRay Support
+  void LowerPATCHABLE_FUNCTION_ENTER(const MachineInstr *MI);
+  void LowerPATCHABLE_FUNCTION_EXIT(const MachineInstr *MI);
+  void LowerPATCHABLE_TAIL_CALL(const MachineInstr *MI);
+
   void emitStartOfAsmFile(Module &M) override;
   void emitEndOfAsmFile(Module &M) override;
 
@@ -93,6 +99,8 @@
 
 private:
   void emitAttributes();
+  // XRay Support
+  void emitSled(const MachineInstr *MI, SledKind Kind);
 
   void emitNTLHint(const MachineInstr *MI);
 
@@ -167,6 +175,27 @@
   case RISCV::PseudoRVVInitUndefM4:
   case RISCV::PseudoRVVInitUndefM8:
 return;
+  case TargetOpcode::PATCHABLE_FUNCTION_ENTER: {
+// This switch case section is only for handling XRay sleds.
+//
+// patchable-function-entry is handled in lowerRISCVMachineInstrToMCInst
+// Therefore, we break out of the switch statement if we encounter it here.
+const Function &F = MI->getParent()->getParent()->getFunction();
+if (F.hasFnAttribute("patchable-function-entry")) {
+  break;
+}
+
+LowerPATCHABLE_FUNCTION_ENTER(MI);
+return;
+  }
+  case TargetOpcode:

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

[Ashwin Poduval via Phabricator via cfe-commits]

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/xray/xray_interface.cpp
  compiler-rt/lib/xray/xray_riscv.cpp
  compiler-rt/lib/xray/xray_trampoline_riscv32.S
  compiler-rt/lib/xray/xray_trampoline_riscv64.S
  compiler-rt/lib/xray/xray_tsc.h
  llvm/lib/CodeGen/XRayInstrumentation.cpp
  llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
  llvm/lib/Target/RISCV/RISCVSubtarget.h
  llvm/test/CodeGen/RISCV/xray-attribute-instrumentation.ll

Index: llvm/test/CodeGen/RISCV/xray-attribute-instrumentation.ll
===
--- /dev/null
+++ llvm/test/CodeGen/RISCV/xray-attribute-instrumentation.ll
@@ -0,0 +1,24 @@
+; RUN: llc -mtriple=riscv32-unknown-linux-gnu -mattr=+d -verify-machineinstrs < %s | FileCheck --check-prefix=CHECK %s
+; RUN: llc -mtriple=riscv64-unknown-linux-gnu -mattr=+d -verify-machineinstrs < %s | FileCheck --check-prefix=CHECK --check-prefix=CHECK-RISCV64 %s
+
+define i32 @foo() nounwind "function-instrument"="xray-always" {
+; CHECK: .p2align 2
+; CHECK-LABEL:   .Lxray_sled_0:
+; CHECK-NEXT:j .Ltmp0
+; CHECK-COUNT-14:nop
+; CHECK-RISCV64-COUNT-4: nop
+; CHECK-LABEL:   .Ltmp0:
+  ret i32 0
+; CHECK: .p2align 2
+; CHECK-LABEL:   .Lxray_sled_1:
+; CHECK-NEXT:j .Ltmp1
+; CHECK-COUNT-14:nop
+; CHECK-RISCV64-COUNT-4: nop
+; CHECK-LABEL:   .Ltmp1:
+; CHECK-NEXT:ret
+}
+; CHECK: .section xray_instr_map,"ao",@progbits,foo
+; CHECK-LABEL:   .Lxray_sleds_start0:
+; CHECK: .Lxray_sled_0-[[TMP:.Ltmp[0-9]+]]
+; CHECK: .Lxray_sled_1-[[TMP:.Ltmp[0-9]+]]
+; CHECK-LABEL:   .Lxray_sleds_end0:
Index: llvm/lib/Target/RISCV/RISCVSubtarget.h
===
--- llvm/lib/Target/RISCV/RISCVSubtarget.h
+++ llvm/lib/Target/RISCV/RISCVSubtarget.h
@@ -160,6 +160,11 @@
 assert(i < RISCV::NUM_TARGET_REGS && "Register out of range");
 return UserReservedRegister[i];
   }
+  // Add XRay support - needs double precision floats at present and does not
+  // support compressed instructions
+  bool isXRaySupported() const override {
+return hasStdExtD() && !hasStdExtC();
+  }
 
   bool hasMacroFusion() const { return hasLUIADDIFusion(); }
 
Index: llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
===
--- llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
+++ llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
@@ -16,6 +16,7 @@
 #include "MCTargetDesc/RISCVMCExpr.h"
 #include "MCTargetDesc/RISCVTargetStreamer.h"
 #include "RISCV.h"
+#include "RISCVSubtarget.h"
 #include "RISCVMachineFunctionInfo.h"
 #include "RISCVTargetMachine.h"
 #include "TargetInfo/RISCVTargetInfo.h"
@@ -84,6 +85,11 @@
   // Wrapper needed for tblgenned pseudo lowering.
   bool lowerOperand(const MachineOperand &MO, MCOperand &MCOp) const;
 
+  // XRay Support
+  void LowerPATCHABLE_FUNCTION_ENTER(const MachineInstr *MI);
+  void LowerPATCHABLE_FUNCTION_EXIT(const MachineInstr *MI);
+  void LowerPATCHABLE_TAIL_CALL(const MachineInstr *MI);
+
   void emitStartOfAsmFile(Module &M) override;
   void emitEndOfAsmFile(Module &M) override;
 
@@ -93,6 +99,8 @@
 
 private:
   void emitAttributes();
+  // XRay Support
+  void emitSled(const MachineInstr *MI, SledKind Kind);
 
   void emitNTLHint(const MachineInstr *MI);
 
@@ -167,6 +175,27 @@
   case RISCV::PseudoRVVInitUndefM4:
   case RISCV::PseudoRVVInitUndefM8:
 return;
+  case TargetOpcode::PATCHABLE_FUNCTION_ENTER: {
+// This switch case section is only for handling XRay sleds.
+//
+// patchable-function-entry is handled in lowerToMCInst
+// Therefore, we break out of the switch statement if we encounter it here.
+const Function &F = MI->getParent()->getParent()->getFunction();
+if (F.hasFnAttribute("patchable-function-entry")) {
+  break;
+}
+
+LowerPATCHABLE_FUNCTION_ENTER(MI);
+return;
+  }
+  case TargetOpcode::PATCHABLE_FUNCTION_EXIT: {
+LowerPATCHABLE_FUNCTION_EXIT(MI);
+return;
+  }
+  case TargetOpcode::PATCHABLE_TAIL_CALL: {
+LowerPATCHABLE_TAIL_CALL(MI);
+return;
+  }
   }
 
   MCInst OutInst;
@@ -282,11 +311,68 @@
   SetupMachineFunction(MF);
   emitFunctionBody();
 
+  // Emit the XRay table
+  emitXRayTable();
+
   if (!isSameAttribute())
 RTS.emitDirectiveOptionPop();
   return false;
 }
 
+void RISCVAsmPrinter::LowerPATCHABLE_FUNCTION_ENTER(const MachineInstr *MI) {
+  emitSled(MI, SledKind::FUNCTION_ENTER);
+}
+
+void RISCVAsmPrinter::LowerPATCHABLE_FUNCTION_EXIT(const MachineInstr *MI) {
+  emitSled(MI, SledKind::FUNCTION_EXIT);
+}
+
+void RISCVAsmPrinter::LowerPATCHABL

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

[Ashwin Poduval via Phabricator via cfe-commits]

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} ${LOONGARCH64} #[[${RISCV32}]] 
${RISCV64})
 endif()

MaskRay wrote:
> Just make RISCV32 available? Otherwise we wouldn't need `riscv32_SOURCES`
Done



Comment at: compiler-rt/lib/xray/xray_riscv.cpp:51
+encodeRTypeInstruction(uint32_t Opcode, uint32_t Rs1, uint32_t Rs2,
+   uint32_t Rd) XRAY_NEVER_INSTRUMENT {
+  return (Rs2 << 20 | Rs1 << 15 | Rd << 7 | Opcode);

MaskRay wrote:
> For new functions, consider dropping `XRAY_NEVER_INSTRUMENT`. The runtime 
> library cannot be instrumented with `-fxray-instrument` and I an unsure why 
> the original impl adds a lot of `XRAY_NEVER_INSTRUMENT`.
Done



Comment at: compiler-rt/lib/xray/xray_riscv.cpp:52
+   uint32_t Rd) XRAY_NEVER_INSTRUMENT {
+  return (Rs2 << 20 | Rs1 << 15 | Rd << 7 | Opcode);
+}

MaskRay wrote:
> 
Done



Comment at: compiler-rt/lib/xray/xray_riscv.cpp:286
+  const XRaySledEntry &Sled) XRAY_NEVER_INSTRUMENT {
+  // FIXME: Implement for riscv?
+  return false;

MaskRay wrote:
> Unimplemented features should use TODO instead of FIXME.
Done



Comment at: compiler-rt/lib/xray/xray_riscv.cpp:162
+uint32_t HighestTracingHookAddr =
+(reinterpret_cast(TracingHook + 0x800) >> 44) & 0xf;
+// We typecast the Tracing Hook to a 32 bit value for RISCV32

MaskRay wrote:
> ashwin98 wrote:
> > jrtc27 wrote:
> > > This is definitely wrong; you probably mean `(((TracingHook >> 32) + 
> > > 0x800) >> 12) & 0xf`?
> > True
> `(x + 0x800) >> 12` is used many times. We need a helper like 
> `lld/ELF/Arch/RISCV.cpp hi20`. Ditto for lo12.
Done. I think I'd frequently looked at the MIPS trampolines while writing this 
code - I noticed that they were casting the Tracing Hook to int64_t, instead of 
uint64_t (and int32/uint32 for the 32 bit ISA), which requires the & operation, 
since >> is an arithmetic shift operation for signed integers. Changing the 
cast operations to uint64_t should eliminate those & operations without 
breaking anything else, right?



Comment at: llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp:322
 
+void RISCVAsmPrinter::LowerPATCHABLE_FUNCTION_ENTER(const MachineInstr *MI) {
+  emitSled(MI, SledKind::FUNCTION_ENTER);

MaskRay wrote:
> I have many comments in D140727 (LoongArch port). Many are probably useful 
> here as well.
I had a look at this. Two things which stuck out to me (both in xray_riscv.cpp):
- changing inline static to static inline (done)
- your note about the PO_ style hurting readability when instructions are only 
used once. In this case, we're using some instructions repeatedly, so I'm 
guessing it makes sense to continue with the enum, but I can get rid of it if 
that works better.


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

https://reviews.llvm.org/D117929

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Ashwin Poduval via Phabricator via cfe-commits]

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.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/xray/xray_interface.cpp
  compiler-rt/lib/xray/xray_riscv.cpp
  compiler-rt/lib/xray/xray_trampoline_riscv32.S
  compiler-rt/lib/xray/xray_trampoline_riscv64.S
  compiler-rt/lib/xray/xray_tsc.h
  llvm/lib/CodeGen/XRayInstrumentation.cpp
  llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
  llvm/lib/Target/RISCV/RISCVSubtarget.h
  llvm/test/CodeGen/RISCV/xray-attribute-instrumentation.ll

Index: llvm/test/CodeGen/RISCV/xray-attribute-instrumentation.ll
===
--- /dev/null
+++ llvm/test/CodeGen/RISCV/xray-attribute-instrumentation.ll
@@ -0,0 +1,24 @@
+; RUN: llc -mtriple=riscv32-unknown-linux-gnu -mattr=+d -verify-machineinstrs < %s | FileCheck --check-prefix=CHECK %s
+; RUN: llc -mtriple=riscv64-unknown-linux-gnu -mattr=+d -verify-machineinstrs < %s | FileCheck --check-prefix=CHECK --check-prefix=CHECK-RISCV64 %s
+
+define i32 @foo() nounwind "function-instrument"="xray-always" {
+; CHECK:.p2align 2
+; CHECK-LABEL:  .Lxray_sled_0:
+; CHECK-NEXT:   j .Ltmp0
+; CHECK-COUNT-14:   nop
+; CHECK-RISCV64-COUNT-4:nop
+; CHECK-LABEL:  .Ltmp0:
+  ret i32 0
+; CHECK:.p2align 2
+; CHECK-LABEL:  .Lxray_sled_1:
+; CHECK-NEXT:   j .Ltmp1
+; CHECK-COUNT-14:   nop
+; CHECK-RISCV64-COUNT-4:nop
+; CHECK-LABEL:  .Ltmp1:
+; CHECK-NEXT:   ret
+}
+; CHECK:.section xray_instr_map,"ao",@progbits,foo
+; CHECK-LABEL:  .Lxray_sleds_start0:
+; CHECK:.Lxray_sled_0-[[TMP:.Ltmp[0-9]+]]
+; CHECK:.Lxray_sled_1-[[TMP:.Ltmp[0-9]+]]
+; CHECK-LABEL:  .Lxray_sleds_end0:
Index: llvm/lib/Target/RISCV/RISCVSubtarget.h
===
--- llvm/lib/Target/RISCV/RISCVSubtarget.h
+++ llvm/lib/Target/RISCV/RISCVSubtarget.h
@@ -160,6 +160,11 @@
 assert(i < RISCV::NUM_TARGET_REGS && "Register out of range");
 return UserReservedRegister[i];
   }
+  // Add XRay support - needs double precision floats at present and does not
+  // support compressed instructions
+  bool isXRaySupported() const override {
+return hasStdExtD() && !hasStdExtC();
+  }
 
   bool hasMacroFusion() const { return hasLUIADDIFusion(); }
 
Index: llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
===
--- llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
+++ llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
@@ -16,6 +16,7 @@
 #include "MCTargetDesc/RISCVMCExpr.h"
 #include "MCTargetDesc/RISCVTargetStreamer.h"
 #include "RISCV.h"
+#include "RISCVSubtarget.h"
 #include "RISCVMachineFunctionInfo.h"
 #include "RISCVTargetMachine.h"
 #include "TargetInfo/RISCVTargetInfo.h"
@@ -84,6 +85,11 @@
   // Wrapper needed for tblgenned pseudo lowering.
   bool lowerOperand(const MachineOperand &MO, MCOperand &MCOp) const;
 
+  // XRay Support
+  void LowerPATCHABLE_FUNCTION_ENTER(const MachineInstr *MI);
+  void LowerPATCHABLE_FUNCTION_EXIT(const MachineInstr *MI);
+  void LowerPATCHABLE_TAIL_CALL(const MachineInstr *MI);
+
   void emitStartOfAsmFile(Module &M) override;
   void emitEndOfAsmFile(Module &M) override;
 
@@ -93,6 +99,8 @@
 
 private:
   void emitAttributes();
+  // XRay Support
+  void emitSled(const MachineInstr *MI, SledKind Kind);
 
   void emitNTLHint(const MachineInstr *MI);
 
@@ -167,6 +175,27 @@
   case RISCV::PseudoRVVInitUndefM4:
   case RISCV::PseudoRVVInitUndefM8:
 return;
+  case TargetOpcode::PATCHABLE_FUNCTION_ENTER: {
+// This switch case section is only for handling XRay sleds.
+//
+// patchable-function-entry is handled in lowerToMCInst
+// Therefore, we break out of the switch statement if we encounter it here.
+const Function &F = MI->getParent()->getParent()->getFunction();
+if (F.hasFnAttribute("patchable-function-entry")) {
+  break;
+}
+
+LowerPATCHABLE_FUNCTION_ENTER(MI);
+return;
+  }
+  case TargetOpcode::PATCHABLE_FUNCTION_EXIT: {
+LowerPATCHABLE_FUNCTION_EXIT(MI);
+return;
+  }
+  case TargetOpcode::PATCHABLE_TAIL_CALL: {
+LowerPATCHABLE_TAIL_CALL(MI);
+return;
+  }
   }
 
   MCInst OutInst;
@@ -282,11 +311,68 @@
   SetupMachineFunction(MF);
   emitFunctionBody();
 
+  // Emit the XRay table
+  emitXRayTable();
+
   if (!isSameAttribute())
 RTS.emitDire

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

[Ashwin Poduval via Phabricator via cfe-commits]

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 identical to what I'd previously observed when 
compiling with march=rv64g. When I had originally written this in 2021/early 
2022, some of the instructions were getting corrupted when they were being 
patched in - either this was a bug in my implementation that has been fixed 
over the course of the review process, or it might've been an issue with qemu 
or llvm at the time that has been fixed in the years since.


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/xray/xray_interface.cpp
  compiler-rt/lib/xray/xray_riscv.cpp
  compiler-rt/lib/xray/xray_trampoline_riscv32.S
  compiler-rt/lib/xray/xray_trampoline_riscv64.S
  compiler-rt/lib/xray/xray_tsc.h
  llvm/lib/CodeGen/XRayInstrumentation.cpp
  llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
  llvm/lib/Target/RISCV/RISCVSubtarget.h
  llvm/test/CodeGen/RISCV/xray-attribute-instrumentation.ll

Index: llvm/test/CodeGen/RISCV/xray-attribute-instrumentation.ll
===
--- /dev/null
+++ llvm/test/CodeGen/RISCV/xray-attribute-instrumentation.ll
@@ -0,0 +1,24 @@
+; RUN: llc -mtriple=riscv32-unknown-linux-gnu -mattr=+d -verify-machineinstrs < %s | FileCheck --check-prefix=CHECK %s
+; RUN: llc -mtriple=riscv64-unknown-linux-gnu -mattr=+d -verify-machineinstrs < %s | FileCheck --check-prefix=CHECK --check-prefix=CHECK-RISCV64 %s
+
+define i32 @foo() nounwind "function-instrument"="xray-always" {
+; CHECK:.p2align 2
+; CHECK-LABEL:  .Lxray_sled_0:
+; CHECK-NEXT:   j .Ltmp0
+; CHECK-COUNT-14:   nop
+; CHECK-RISCV64-COUNT-4:nop
+; CHECK-LABEL:  .Ltmp0:
+  ret i32 0
+; CHECK:.p2align 2
+; CHECK-LABEL:  .Lxray_sled_1:
+; CHECK-NEXT:   j .Ltmp1
+; CHECK-COUNT-14:   nop
+; CHECK-RISCV64-COUNT-4:nop
+; CHECK-LABEL:  .Ltmp1:
+; CHECK-NEXT:   ret
+}
+; CHECK:.section xray_instr_map,"ao",@progbits,foo
+; CHECK-LABEL:  .Lxray_sleds_start0:
+; CHECK:.Lxray_sled_0-[[TMP:.Ltmp[0-9]+]]
+; CHECK:.Lxray_sled_1-[[TMP:.Ltmp[0-9]+]]
+; CHECK-LABEL:  .Lxray_sleds_end0:
Index: llvm/lib/Target/RISCV/RISCVSubtarget.h
===
--- llvm/lib/Target/RISCV/RISCVSubtarget.h
+++ llvm/lib/Target/RISCV/RISCVSubtarget.h
@@ -160,6 +160,10 @@
 assert(i < RISCV::NUM_TARGET_REGS && "Register out of range");
 return UserReservedRegister[i];
   }
+  // Add XRay support - assume D and C extensions available
+  bool isXRaySupported() const override {
+return hasStdExtD() && hasStdExtC();
+  }
 
   bool hasMacroFusion() const { return hasLUIADDIFusion(); }
 
Index: llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
===
--- llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
+++ llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
@@ -16,6 +16,7 @@
 #include "MCTargetDesc/RISCVMCExpr.h"
 #include "MCTargetDesc/RISCVTargetStreamer.h"
 #include "RISCV.h"
+#include "RISCVSubtarget.h"
 #include "RISCVMachineFunctionInfo.h"
 #include "RISCVTargetMachine.h"
 #include "TargetInfo/RISCVTargetInfo.h"
@@ -84,6 +85,11 @@
   // Wrapper needed for tblgenned pseudo lowering.
   bool lowerOperand(const MachineOperand &MO, MCOperand &MCOp) const;
 
+  // XRay Support
+  void LowerPATCHABLE_FUNCTION_ENTER(const MachineInstr *MI);
+  void LowerPATCHABLE_FUNCTION_EXIT(const MachineInstr *MI);
+  void LowerPATCHABLE_TAIL_CALL(const MachineInstr *MI);
+
   void emitStartOfAsmFile(Module &M) override;
   void emitEndOfAsmFile(Module &M) override;
 
@@ -93,6 +99,8 @@
 
 private:
   void emitAttributes();
+  // XRay Support
+  void emitSled(const MachineInstr *MI, SledKind Kind);
 
   void emitNTLHint(const MachineInstr *MI);
 
@@ -167,6 +175,26 @@
   case RISCV::PseudoRVVInitUndefM4:
   case RISCV::PseudoRVVInitUndefM8:
 return;
+  case TargetOpcode::PATCHABLE_FUNCTION_ENTER: {
+// This switch case section is only for handling XRay sleds.
+//
+// patchable-function-entry is handled in lowerToMCInst
+// Therefore, we break out of the switch statement if we encounter it here.
+const Function &F = MI->getParent()->getParent()->getFunction();
+if (F.hasFnAttribute("patchable-function-entry"))
+  break;
+
+LowerPATCHABLE_FUNCTION_ENTER(MI);
+return;
+  }

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

[Ashwin Poduval via Phabricator via cfe-commits]

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 just use numbers (`1f` and 
> `1:`) or `LOCAL_LABEL(...)`, so that the symbol table will not have the 
> unneeded symbol entries.
Done



Comment at: compiler-rt/lib/xray/xray_trampoline_riscv32.S:85
+FunctionEntry_end:
+   ASM_SIZE(__xray_FunctionEntry)
+   .cfi_endproc

MaskRay wrote:
> stray `ASM_SIZE`?
Fixed - I looked at your AArch64 code and tried to follow the same process here



Comment at: llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp:185
+if (F.hasFnAttribute("patchable-function-entry")) {
+  break;
+}

MaskRay wrote:
> omit braces for single-line single statement body.
Done


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

https://reviews.llvm.org/D117929

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Ashwin Poduval via Phabricator via cfe-commits]

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. Should 
hopefully be able to get back to this by some time in August.


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

https://reviews.llvm.org/D117929

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Ashwin Poduval via Phabricator via cfe-commits]

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, rogfer01, 
edward-jones, zzheng, jrtc27, kito-cheng, niosHD, sabuasal, simoncook, 
johnrusso, rbar, hiraditya, mgorny.
ashwin98 requested review of this revision.
Herald added subscribers: llvm-commits, Sanitizers, cfe-commits, eopXD, MaskRay.
Herald added projects: clang, Sanitizers, LLVM.

Adds XRay support for RISCV. Currently, only RISCV64 has been tested. Changes 
required to add support for RISCV32 are present, but are commented out.

The modifications (as in the case of RISCVAsmPrinter) were made with the 
intention of making as few changes/touching as few files as possible to reduce 
the possibility of breaking any other functionality.

The double precision floating point extension was assumed to be part of the ISA 
while designing the trampolines (and this has been enforced in the Subtarget 
file). This implementation currently does not support compressed instructions.


Repository:
  rG LLVM Github Monorepo

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/xray/xray_interface.cpp
  compiler-rt/lib/xray/xray_riscv32.cpp
  compiler-rt/lib/xray/xray_riscv64.cpp
  compiler-rt/lib/xray/xray_trampoline_riscv32.S
  compiler-rt/lib/xray/xray_trampoline_riscv64.S
  compiler-rt/lib/xray/xray_tsc.h
  llvm/lib/CodeGen/XRayInstrumentation.cpp
  llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
  llvm/lib/Target/RISCV/RISCVSubtarget.h
  llvm/test/CodeGen/RISCV/xray-attribute-instrumentation.ll

Index: llvm/test/CodeGen/RISCV/xray-attribute-instrumentation.ll
===
--- /dev/null
+++ llvm/test/CodeGen/RISCV/xray-attribute-instrumentation.ll
@@ -0,0 +1,70 @@
+; RUN: llc -filetype=asm -o - -mtriple=riscv32-unknown-elf < %s | FileCheck --check-prefix=CHECK %s
+; RUN: llc -filetype=asm -o - -mtriple=riscv32-unknown-linux-gnu  < %s | FileCheck --check-prefix=CHECK %s
+; RUN: llc -filetype=asm -o - -mtriple=riscv64-unknown-elf < %s | FileCheck --check-prefix=CHECK --check-prefix=CHECK-RISCV64 %s
+; RUN: llc -filetype=asm -o - -mtriple=riscv64-unknown-linux-gnu  < %s | FileCheck --check-prefix=CHECK --check-prefix=CHECK-RISCV64 %s
+
+define i32 @foo() nounwind noinline uwtable "function-instrument"="xray-always" {
+; CHECK:		.p2align 2
+; CHECK-LABEL:		.Lxray_sled_0:
+; CHECK-NEXT:		j .Ltmp0
+; CHECK-NEXT:  		nop
+; CHECK-NEXT:  		nop
+; CHECK-NEXT:  		nop
+; CHECK-NEXT:  		nop
+; CHECK-NEXT:  		nop
+; CHECK-NEXT:  		nop
+; CHECK-NEXT:  		nop
+; CHECK-NEXT:  		nop
+; CHECK-NEXT:  		nop
+; CHECK-NEXT:  		nop
+; CHECK-NEXT:  		nop
+; CHECK-NEXT:  		nop
+; CHECK-NEXT:  		nop
+; CHECK-NEXT:  		nop
+; CHECK-RISCV64:	nop
+; CHECK-RISCV64:	nop
+; CHECK-RISCV64:	nop
+; CHECK-RISCV64:	nop
+; CHECK-RISCV64:	nop
+; CHECK-RISCV64:	nop
+; CHECK-RISCV64:	nop
+; CHECK-RISCV64:	nop
+; CHECK-RISCV64:	nop
+; CHECK-RISCV64:	nop
+; CHECK-LABEL:		.Ltmp0:
+  ret i32 0
+; CHECK:		.p2align 2
+; CHECK-LABEL:		.Lxray_sled_1:
+; CHECK-NEXT:		j .Ltmp1
+; CHECK-NEXT:  		nop
+; CHECK-NEXT:  		nop
+; CHECK-NEXT:  		nop
+; CHECK-NEXT:  		nop
+; CHECK-NEXT:  		nop
+; CHECK-NEXT:  		nop
+; CHECK-NEXT:  		nop
+; CHECK-NEXT:  		nop
+; CHECK-NEXT:  		nop
+; CHECK-NEXT:  		nop
+; CHECK-NEXT:  		nop
+; CHECK-NEXT:  		nop
+; CHECK-NEXT:  		nop
+; CHECK-NEXT:  		nop
+; CHECK-RISCV64:	nop
+; CHECK-RISCV64:	nop
+; CHECK-RISCV64:	nop
+; CHECK-RISCV64:	nop
+; CHECK-RISCV64:	nop
+; CHECK-RISCV64:	nop
+; CHECK-RISCV64:	nop
+; CHECK-RISCV64:	nop
+; CHECK-RISCV64:	nop
+; CHECK-RISCV64:	nop
+; CHECK-LABEL:		.Ltmp1:
+; CHECK-NEXT:  		ret
+}
+; CHECK:		.section xray_instr_map,{{.*}}
+; CHECK-LABEL:		.Lxray_sleds_start0:
+; CHECK:		.Lxray_sled_0-.Ltmp{{.*}}
+; CHECK:		.Lxray_sled_1-.Ltmp{{.*}}
+; CHECK-LABEL:		.Lxray_sleds_end0:
Index: llvm/lib/Target/RISCV/RISCVSubtarget.h
===
--- llvm/lib/Target/RISCV/RISCVSubtarget.h
+++ llvm/lib/Target/RISCV/RISCVSubtarget.h
@@ -187,6 +187,10 @@
   unsigned getMaxInterleaveFactor() const {
 return hasVInstructions() ? MaxInterleaveFactor : 1;
   }
+  // Add XRay support - needs double precision floats at present
+  bool isXRaySupported() const override { 
+return hasStdExtD(); 
+  }
 
 protected:
   // GlobalISel related APIs.
Index: llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
===
--- llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
+++ llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
@@ -16,6 +16,7 @@
 #include "MCTargetDesc/RISCVTargetStreamer.h"
 #include "RISCV.h"
 #include "RISCVTargetMachine.h"
+#include "RISCVSubtarget.h"
 #include "

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

[Ashwin Poduval via Phabricator via cfe-commits]

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 commented riscv32 out because I haven't managed to 
test it out yet, I've had some difficulty getting llvm set up for riscv 32 - I 
will leave riscv32 commented out only in the cmake file.

I wasn't too sure about how to work around sign extension in RISCV, which you 
have picked up on - I agree that adding 0x800 would help deal with sign 
extension, but wouldn't that only be required if the LSBs were sign extended? 
The way I understand it, if we were to always add 0x800, we'd end up adding 
4096 even when our address is a small positive number like 40.

I had a similar question with respect to the 64 bit sleds - given that lui is 
also sign extended, we need a work around for it as well while constructing the 
32 bit values, and while combining the 2 32 bit values into a 64 bit value. I 
have currently been getting rid of the upper 32 bits by performing a left shift 
followed by a right shift, but I'm sure there is a better solution to it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117929

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Ashwin Poduval via Phabricator via cfe-commits]

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 comments in 
RISCVAsmPrinter.cpp
3. Updated the test file to pass -verify-machineinstrs and remove unnecessary 
attributes as well as {{.*}}s
4. Fixed riscv32 comments - it is now only commented out in 
cmake/Modules/AllSupportedArchDefs.cmake

I have been testing this patch on qemu using ubuntu for riscv64, the comment 
that Phabricator detects in the supported architecture definitions cmake file 
is probably an issue with syntax highlighting. Nevertheless, we could instead 
comment out riscv32 in clang/lib/Driver/XRayArgs, which would also throw up an 
error during compilation stating that the target is not supported.


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/xray/xray_interface.cpp
  compiler-rt/lib/xray/xray_riscv.cpp
  compiler-rt/lib/xray/xray_trampoline_riscv32.S
  compiler-rt/lib/xray/xray_trampoline_riscv64.S
  compiler-rt/lib/xray/xray_tsc.h
  llvm/lib/CodeGen/XRayInstrumentation.cpp
  llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
  llvm/lib/Target/RISCV/RISCVSubtarget.h
  llvm/test/CodeGen/RISCV/xray-attribute-instrumentation.ll

Index: llvm/test/CodeGen/RISCV/xray-attribute-instrumentation.ll
===
--- /dev/null
+++ llvm/test/CodeGen/RISCV/xray-attribute-instrumentation.ll
@@ -0,0 +1,70 @@
+; 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 -verify-machineinstrs < %s | FileCheck --check-prefix=CHECK %s
+; RUN: llc -mtriple=riscv64-unknown-elf -mattr=+d -verify-machineinstrs < %s | FileCheck --check-prefix=CHECK --check-prefix=CHECK-RISCV64 %s
+; RUN: llc -mtriple=riscv64-unknown-linux-gnu -mattr=+d -verify-machineinstrs < %s | FileCheck --check-prefix=CHECK --check-prefix=CHECK-RISCV64 %s
+
+define i32 @foo() nounwind "function-instrument"="xray-always" {
+; CHECK:.p2align 2
+; CHECK-LABEL:  .Lxray_sled_0:
+; CHECK-NEXT:   j .Ltmp0
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-LABEL:  .Ltmp0:
+  ret i32 0
+; CHECK:.p2align 2
+; CHECK-LABEL:  .Lxray_sled_1:
+; CHECK-NEXT:   j .Ltmp1
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-LABEL:  .Ltmp1:
+; CHECK-NEXT:   ret
+}
+; CHECK:.section xray_instr_map,"ao",@progbits,foo
+; CHECK-LABEL:  .Lxray_sleds_start0:
+; CHECK:.Lxray_sled_0-.Ltmp2
+; CHECK:.Lxray_sled_1-.Ltmp3
+; CHECK-LABEL:  .Lxray_sleds_end0:
Index: llvm/lib/Target/RISCV/RISCVSubtarget.h
===
--- llvm/lib/Target/RISCV/RISCVSubtarget.h
+++ llvm/lib/Target/RISCV/RISCVSubtarget.h
@@ -187,6 +187,8 @@
   unsigned getMaxInterleaveFactor() const {
 return hasVInstructions() ? MaxInterleaveFactor : 1;
   }
+  // Add XRay support - needs double precision floats at present
+  bool isXRaySupported() const override { return hasStdExtD(); }
 
 protected:
   // GlobalISel related APIs.
Index: llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
===
--- llvm/lib/Target/RI

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

[Ashwin Poduval via Phabricator via cfe-commits]

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/AllSupportedArchDefs.cmake
  compiler-rt/lib/xray/CMakeLists.txt
  compiler-rt/lib/xray/xray_interface.cpp
  compiler-rt/lib/xray/xray_riscv.cpp
  compiler-rt/lib/xray/xray_trampoline_riscv32.S
  compiler-rt/lib/xray/xray_trampoline_riscv64.S
  compiler-rt/lib/xray/xray_tsc.h
  llvm/lib/CodeGen/XRayInstrumentation.cpp
  llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
  llvm/lib/Target/RISCV/RISCVSubtarget.h
  llvm/test/CodeGen/RISCV/xray-attribute-instrumentation.ll

Index: llvm/test/CodeGen/RISCV/xray-attribute-instrumentation.ll
===
--- /dev/null
+++ llvm/test/CodeGen/RISCV/xray-attribute-instrumentation.ll
@@ -0,0 +1,70 @@
+; 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 -verify-machineinstrs < %s | FileCheck --check-prefix=CHECK %s
+; RUN: llc -mtriple=riscv64-unknown-elf -mattr=+d -verify-machineinstrs < %s | FileCheck --check-prefix=CHECK --check-prefix=CHECK-RISCV64 %s
+; RUN: llc -mtriple=riscv64-unknown-linux-gnu -mattr=+d -verify-machineinstrs < %s | FileCheck --check-prefix=CHECK --check-prefix=CHECK-RISCV64 %s
+
+define i32 @foo() nounwind "function-instrument"="xray-always" {
+; CHECK:.p2align 2
+; CHECK-LABEL:  .Lxray_sled_0:
+; CHECK-NEXT:   j .Ltmp0
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-LABEL:  .Ltmp0:
+  ret i32 0
+; CHECK:.p2align 2
+; CHECK-LABEL:  .Lxray_sled_1:
+; CHECK-NEXT:   j .Ltmp1
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-LABEL:  .Ltmp1:
+; CHECK-NEXT:   ret
+}
+; CHECK:.section xray_instr_map,"ao",@progbits,foo
+; CHECK-LABEL:  .Lxray_sleds_start0:
+; CHECK:.Lxray_sled_0-.Ltmp2
+; CHECK:.Lxray_sled_1-.Ltmp3
+; CHECK-LABEL:  .Lxray_sleds_end0:
Index: llvm/lib/Target/RISCV/RISCVSubtarget.h
===
--- llvm/lib/Target/RISCV/RISCVSubtarget.h
+++ llvm/lib/Target/RISCV/RISCVSubtarget.h
@@ -187,6 +187,9 @@
   unsigned getMaxInterleaveFactor() const {
 return hasVInstructions() ? MaxInterleaveFactor : 1;
   }
+  // Add XRay support - needs double precision floats at present and does not
+  // support compressed instructions
+  bool isXRaySupported() const override { return hasStdExtD() && !hasStdExtC(); }
 
 protected:
   // GlobalISel related APIs.
Index: llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
===
--- llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
+++ llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
@@ -15,6 +15,7 @@
 #include "MCTargetDesc/RISCVMCExpr.h"
 #include "MCTargetDesc/RISCVTargetStreamer.h"
 #include "RISCV.h"
+#include "RISCVSubtarget.h"
 #include "RISCVTargetMachine.h"
 #include "TargetInfo/RISCVTargetInfo.h"
 #include "llvm/ADT/Statistic.h"
@@ -25,10 +26,12 @@
 #include "llvm/CodeGen/MachineModuleInfo.h"
 #include "llvm/MC/MCAsmInfo.h"
 #include "llvm/MC/MCInst.h"
+#include "llvm/MC/MCInstBuilder.h"
 #include "llvm/MC/MCStreamer.h"
 #include "llvm/MC/MCSymbol.h"
 #include "llvm/MC/TargetRegistry.h"
 #include "llvm/Support/raw_ostream.h"
+#include 
 using namespace llvm;
 
 #define DEBUG_TYPE "asm-printer"
@@ -65,11 +68,18 @@
 return LowerRISCVMach

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

[Ashwin Poduval via Phabricator via cfe-commits]

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 add 
new ones for the llvm-xray tool under llvm/test/tools?


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/xray/xray_interface.cpp
  compiler-rt/lib/xray/xray_riscv.cpp
  compiler-rt/lib/xray/xray_trampoline_riscv32.S
  compiler-rt/lib/xray/xray_trampoline_riscv64.S
  compiler-rt/lib/xray/xray_tsc.h
  llvm/lib/CodeGen/XRayInstrumentation.cpp
  llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
  llvm/lib/Target/RISCV/RISCVSubtarget.h
  llvm/test/CodeGen/RISCV/xray-attribute-instrumentation.ll

Index: llvm/test/CodeGen/RISCV/xray-attribute-instrumentation.ll
===
--- /dev/null
+++ llvm/test/CodeGen/RISCV/xray-attribute-instrumentation.ll
@@ -0,0 +1,70 @@
+; 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 -verify-machineinstrs < %s | FileCheck --check-prefix=CHECK %s
+; RUN: llc -mtriple=riscv64-unknown-elf -mattr=+d -verify-machineinstrs < %s | FileCheck --check-prefix=CHECK --check-prefix=CHECK-RISCV64 %s
+; RUN: llc -mtriple=riscv64-unknown-linux-gnu -mattr=+d -verify-machineinstrs < %s | FileCheck --check-prefix=CHECK --check-prefix=CHECK-RISCV64 %s
+
+define i32 @foo() nounwind "function-instrument"="xray-always" {
+; CHECK:.p2align 2
+; CHECK-LABEL:  .Lxray_sled_0:
+; CHECK-NEXT:   j .Ltmp0
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-LABEL:  .Ltmp0:
+  ret i32 0
+; CHECK:.p2align 2
+; CHECK-LABEL:  .Lxray_sled_1:
+; CHECK-NEXT:   j .Ltmp1
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-LABEL:  .Ltmp1:
+; CHECK-NEXT:   ret
+}
+; CHECK:.section xray_instr_map,"ao",@progbits,foo
+; CHECK-LABEL:  .Lxray_sleds_start0:
+; CHECK:.Lxray_sled_0-.Ltmp2
+; CHECK:.Lxray_sled_1-.Ltmp3
+; CHECK-LABEL:  .Lxray_sleds_end0:
Index: llvm/lib/Target/RISCV/RISCVSubtarget.h
===
--- llvm/lib/Target/RISCV/RISCVSubtarget.h
+++ llvm/lib/Target/RISCV/RISCVSubtarget.h
@@ -187,6 +187,11 @@
   unsigned getMaxInterleaveFactor() const {
 return hasVInstructions() ? MaxInterleaveFactor : 1;
   }
+  // Add XRay support - needs double precision floats at present and does not
+  // support compressed instructions
+  bool isXRaySupported() const override {
+return hasStdExtD() && !hasStdExtC();
+  }
 
 protected:
   // GlobalISel related APIs.
Index: llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
===
--- llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
+++ llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
@@ -15,6 +15,7 @@
 #include "MCTargetDesc/RISCVMCExpr.h"
 #include "MCTargetDesc/RISCVTargetStreamer.h"
 #include "RISCV.h"
+#include "RISCVSubtarget.h"
 #include "RISCVTargetMachine.h"
 #include "TargetInfo/RISCVTargetInfo.h"
 #include "llvm/ADT/Statistic.h"
@@ -25,10 +26,12 @@
 #include "llvm/CodeGen/MachineModuleInfo.h"
 #include "llvm/MC/MCAsmInfo.h"
 #include "llvm/MC/MCInst.h"
+#include "llvm/MC/MCInstBuilder.h"

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

[Ashwin Poduval via Phabricator via cfe-commits]

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 ;set upper 32 bits to 0
+  //addi t2, t2, %higher(__xray_FunctionEntry/Exit)

jrtc27 wrote:
> Why? You shift this whole thing left by 32 later
Right, I'll update this



Comment at: compiler-rt/lib/xray/xray_riscv.cpp:127-128
+  //lui t1, t1, %hi(__xray_FunctionEntry/Exit)
+  //slli t1, t1, 32 ;lui sign extends 
values
+  //srli t1, t1, 32 ;set upper 32 bits to 0
+  //addi t1, t1, %lo(__xray_FunctionEntry/Exit)

jrtc27 wrote:
> You might be able to avoid this by adding 0x8800 before computing 
> "%higher" and "%highest" (feels rather MIPSy... not official things)? Not 
> sure, would need to think this through more, but it feels like it should be 
> possible...
Adding 0x8000 may be enough, the lower 12 bits should be taken care of when 
we construct the lower 32 bits, if we choose to use two registers. If we wish 
to use one register to load all values, then 0x8800 may be needed - I'm not 
too sure. About the MIPS and AArch terminology in some places - yeah if there's 
anything that is not official or consistent with RISCV, please let me know, I 
frequently consulted the files for the other ISAs to figure out XRay's 
implementation and ended up using inconsistent terminology at some places.



Comment at: compiler-rt/lib/xray/xray_riscv.cpp:162
+uint32_t HighestTracingHookAddr =
+(reinterpret_cast(TracingHook + 0x800) >> 44) & 0xf;
+// We typecast the Tracing Hook to a 32 bit value for RISCV32

jrtc27 wrote:
> This is definitely wrong; you probably mean `(((TracingHook >> 32) + 0x800) 
> >> 12) & 0xf`?
True


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

https://reviews.llvm.org/D117929

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits