[PATCH] D96099: [AArch64][GlobalISel] Support the 'returned' parameter attribute.

Jessica Paquette via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Feb 5 09:43:51 PST 2021


paquette added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/GlobalISel/CallLowering.h:271
+      ValueHandler &Handler,
+      std::function<bool(EVT)> TypeIsValidForThisReturn =
+          [](EVT T) { return false; },
----------------
Can we document this parameter?


================
Comment at: llvm/lib/CodeGen/GlobalISel/CallLowering.cpp:174
+
+  if (Flags.isSwiftSelf())
+    Flags.setReturned(false);
----------------
Comment would be useful here


================
Comment at: llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp:1012
 
+  bool UsingReturnedArg = !OutArgs.empty() && OutArgs[0].Flags[0].isReturned();
+  if (UsingReturnedArg) {
----------------
Would it make sense to make this a function, since it's the same as the code in `lowerTailCall`?

(The tail call/standard call code really needs to be refactored, so maybe it's better to leave it. I'm not sure.)


================
Comment at: llvm/test/CodeGen/AArch64/arm64-this-return.ll:2
 ; RUN: llc < %s -mtriple=arm64-eabi | FileCheck %s
+; RUN: llc < %s -mtriple=arm64-eabi -global-isel -global-isel-abort=1 | FileCheck %s
 
----------------
Would it make sense to add a MIR test for this as well? Or would that just be unnecessary test duplication?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96099



More information about the llvm-commits mailing list