[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