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

[Pete Steinfeld via Phabricator via llvm-commits]

PeteSteinfeld added inline comments.


================
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};
----------------
jeanPerier wrote:
> 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.
I'm not sure what previous problem I was running into, but my latest implementation doesn't require a change to the interface.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106292