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 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...
Adding 0x80000000 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 0x80000800 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<int64_t>(TracingHook + 0x800) >> 44) & 0xfffff;
+    // 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) & 0xfffff`?
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

Reply via email to