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

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 29 21:14:40 PDT 2023

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

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

Comment at: compiler-rt/lib/xray/xray_trampoline_riscv64.S:22
+	ASM_TYPE_FUNCTION(__xray_FunctionEntry)
+	ASM_SYMBOL(__xray_FunctionEntry):
+	.cfi_startproc
See the aarch64 implementation to use `ASM_SIZE(__xray_FunctionEntry)` to set `.size`

Comment at: llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp:185
+    if (F.hasFnAttribute("patchable-function-entry")) {
+      break;
+    }
omit braces for single-line single statement body.

Comment at: llvm/lib/Target/RISCV/RISCVSubtarget.h:164
+  // Add XRay support - needs double precision floats at present and does not
+  // support compressed instructions
+  bool isXRaySupported() const override {
"does not support compressed instructions" makes this feature infeasible for many systems, I'll say probably almost all systems that may consider XRay. 



More information about the llvm-commits mailing list