[PATCH] D106292: [flang] Implement the runtime portion of the CSHIFT intrinsic

[Jean Perier via Phabricator via llvm-commits]

jeanPerier accepted this revision.
jeanPerier added inline comments.
This revision is now accepted and ready to land.


================
Comment at: flang/runtime/transformational.cpp:177
 void RTNAME(CshiftVector)(Descriptor &result, const Descriptor &source,
-    std::int64_t shift, const char *sourceFile, int line) {
+    const Descriptor &shift, const char *sourceFile, int line) {
   Terminator terminator{sourceFile, line};
----------------
I am shared. I understand why you are doing this interface change to have simpler lowering code, but using a descriptor instead of a simple integer has user runtime cost (creating the descriptor and reading it).
In this specific case, the extra runtime cost coming from the descriptor might be negligible compared to what CSHIFT is doing, and it's hard for me to complain about performance without any numbers/benchmark. So this change is probably OK.
In general, think we should find solution in lowering to keep lowering code as simple as possible rather than moving the cost to the user.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106292