[PATCH] D158246: [amdgpu] WIP variadics

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/CodeGen/DesugarVariadics.cpp:22
+// 5/ Delete the remaining parts of the original functions
+//
+//===----------------------------------------------------------------------===//
----------------
Can you expand on the ABI requirements of this in the comment? If we make this be the way the ABI works for AMDGPU, we don't need any conditions. Should there be some control for the targets with established ABIs to apply this to internal functions?


================
Comment at: llvm/lib/CodeGen/DesugarVariadics.cpp:38
+
+#include <cstdio>
+
----------------
Don't need cstdio


================
Comment at: llvm/lib/CodeGen/DesugarVariadics.cpp:102-103
+    Value *src = Inst->getSrc();
+    Value *ld = Builder.CreateLoad(src->getType(), src, "vacopy");
+    Builder.CreateStore(ld, dst);
+    Inst->eraseFromParent();
----------------
Can you use nontrivial types with this? Might be better to just start with memcpy


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp:621
         }
+        if (PassName == "desugar-variadics") {
+          PM.addPass(DesugarVariadicsPass());
----------------
desugar is a weird name for a lowering pass


================
Comment at: llvm/test/CodeGen/AMDGPU/llc-pipeline.ll:188
 ; GCN-O1-NEXT:        Cycle Info Analysis
+; GCN-O1-NEXT:    Desugar Variadics
 ; GCN-O1-NEXT:    FunctionPass Manager
----------------
This is rather late, I'd think we'd be better off with an earlier run (e.g. as part of the initial module passes, along with sanitizers)


================
Comment at: llvm/test/CodeGen/AMDGPU/unsupported-calls.ll:57-58
 
 ; GCN: in function test_tail_call_bitcast_extern_variadic{{.*}}: unsupported required tail call to function extern_variadic
-; R600: in function test_tail_call_bitcast_extern_variadic{{.*}}: unsupported call to function extern_variadic
 define i32 @test_tail_call_bitcast_extern_variadic(<4 x float> %arg0, <4 x float> %arg1, i32 %arg2) {
----------------
Deleted the wrong error?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158246