[Openmp-commits] [PATCH] D59880: [WIP] RISCV64 port

[Ferran Pallarès Roca via Phabricator via Openmp-commits]

fpallares added a comment.

Thanks for the quick review @Hahnfeld, I've just replied to your inline comments and updated the patch!



================
Comment at: runtime/test/ompt/callback.h:170
+#elif KMP_ARCH_RISCV64
+// On RV64C the C.NOP instruction is 2 byte long. In addition, the compiler
+// inserts a data-movement instruction for non-void runtime functions which
----------------
Hahnfeld wrote:
> Should this also handle the case that the `nop` is uncompressed (this should be 4 bytes)?
As Linux requires riscv64gc we assumed compressed instructions would be enabled, but certainly it is not necessarily the case (riscv64g should be supported). Good catch!


================
Comment at: runtime/test/ompt/callback.h:172-173
+// inserts a data-movement instruction for non-void runtime functions which
+// accounts for another 2 or 4 bytes. Finally, a C.J instruction may appear
+// (adding 2 more bytes).
+#define print_possible_return_addresses(addr) \
----------------
Hahnfeld wrote:
> Did you try to find out why the compiler inserts a jump?
> And, as above, should be also allow the instruction to be 4 bytes long?
The unnecessary jump to the immediate successor block is removed by FastIsel in the targets that enable it. For now RISCV64 does not.


Repository:
  rOMP OpenMP

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

https://reviews.llvm.org/D59880