[PATCH] D158246: [amdgpu] WIP variadics
Matt Arsenault via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Aug 18 13:25:36 PDT 2023
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
More information about the cfe-commits
mailing list