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 ;set upper 32 bits to 0 + // addi t2, t2, %higher(__xray_FunctionEntry/Exit) ---------------- Why? You shift this whole thing left by 32 later ================ 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) ---------------- You might be able to avoid this by adding 0x80000800 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... ================ Comment at: compiler-rt/lib/xray/xray_riscv.cpp:162 + uint32_t HighestTracingHookAddr = + (reinterpret_cast<int64_t>(TracingHook + 0x800) >> 44) & 0xfffff; + // We typecast the Tracing Hook to a 32 bit value for RISCV32 ---------------- This is definitely wrong; you probably mean `(((TracingHook >> 32) + 0x800) >> 12) & 0xfffff`? ================ Comment at: llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp:105-106 + // Handle XRay sleds while keeping changes minimal to avoid breaking + // functionality + switch (MI->getOpcode()) { ---------------- "Keeping changes minimal" seems like the kind of thing that belongs in a commit message, not a code comment ================ 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 ---------------- 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 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