[PATCH] D158246: [amdgpu] WIP variadics

arsenm added inline comments. Comment at: llvm/lib/CodeGen/DesugarVariadics.cpp:145 +for (Function &F : llvm::make_early_inc_range(M)) + if (Apply || canTransformFunctionInIsolation(F)) +Changed |= runOnFunction(F); I think you need to guard agai

[PATCH] D158246: [amdgpu] WIP variadics

arsenm added inline comments. Comment at: llvm/lib/CodeGen/DesugarVariadics.cpp:296 +// Note - same attribute handling as DeadArgumentElimination +NF->copyAttributesFrom(&F); +NF->setComdat(F.getComdat()); This might be missing copying the linkage R

[PATCH] D158246: [amdgpu] WIP variadics

arsenm added inline comments. Comment at: llvm/lib/CodeGen/DesugarVariadics.cpp:74-77 +Value *Mask = ConstantInt::get(IntPtrTy, ~(DataAlignMinusOne)); +Value *vaListAligned = Builder.CreateIntToPtr( +Builder.CreateAnd(Builder.CreatePtrToInt(Incr, IntPtrTy), Mask),

[PATCH] D158246: [amdgpu] WIP variadics

JonChesterfield added a comment. Name candidates - expand - lower - desugar - transform Lowering probably makes the most sense for the abi level apply to all functions, I like desugar to cover rewriting a subset of the graph Comment at: libc/config/gpu/entrypoints.txt:84-85

[PATCH] D158246: [amdgpu] WIP variadics

arsenm added inline comments. Comment at: llvm/lib/CodeGen/DesugarVariadics.cpp:297 +NF->copyAttributesFrom(&F); +NF->setComdat(F.getComdat()); +F.getParent()->getFunctionList().insert(F.getIterator(), NF); Test the comdat? Weird that copyAttributesFr

[PATCH] D158246: [amdgpu] WIP variadics

arsenm added inline comments. Comment at: llvm/lib/CodeGen/DesugarVariadics.cpp:208-209 + +StructType *VarargsTy = StructType::create( +Ctx, LocalVarTypes, (Twine(NF->getName()) + ".vararg").str()); + Should we go for a packed struct forced to align 4

[PATCH] D158246: [amdgpu] WIP variadics

JonChesterfield removed subscribers: kristof.beyls, wangpc, jdoerfert. JonChesterfield added inline comments. Comment at: llvm/lib/CodeGen/ExpandVAIntrinsics.cpp:215-217 +auto alloced = Builder.Insert( +new AllocaInst(VarargsTy, DL.getAllocaAddrSpace(), nullptr, +

[PATCH] D158246: [amdgpu] WIP variadics

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 requirem

[PATCH] D158246: [amdgpu] WIP variadics

arsenm added inline comments. Comment at: libc/config/gpu/entrypoints.txt:84-85 # stdio.h entrypoints +libc.src.stdio.snprintf +libc.src.stdio.vsnprintf libc.src.stdio.puts Split of the libc stuff into a separate patch, the lowering pass should

[PATCH] D158246: [amdgpu] WIP variadics

JonChesterfield updated this revision to Diff 551616. JonChesterfield added a comment. - Rename ExpandVAIntrinsics to DesugarVariadics Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158246/new/ https://reviews.llvm.org/D158246 Files: clang/lib/Co

[PATCH] D158246: [amdgpu] WIP variadics

arsenm added inline comments. Comment at: llvm/test/CodeGen/Generic/expand-variadic-intrinsics.ll:76 +} + + arsenm wrote: > arsenm wrote: > > Needs some indirect variadic call tests > Also some metadata and signext/zeroext preservation tests Also a case where the

[PATCH] D158246: [amdgpu] WIP variadics

arsenm added inline comments. Comment at: llvm/test/CodeGen/Generic/expand-variadic-intrinsics.ll:76 +} + + arsenm wrote: > Needs some indirect variadic call tests Also some metadata and signext/zeroext preservation tests Repository: rG LLVM Github Monorepo

[PATCH] D158246: [amdgpu] WIP variadics

arsenm added inline comments. Comment at: llvm/lib/CodeGen/ExpandVAIntrinsics.cpp:38 + +#include + Don't need Comment at: llvm/lib/CodeGen/ExpandVAIntrinsics.cpp:44-47 +static cl::opt +ApplyToAllOverride(DEBUG_TYPE "-all", cl::init(false),

[PATCH] D158246: [amdgpu] WIP variadics

JonChesterfield added a comment. @arsenm suggested va_start/va_end should be addrspace aware, similar idea to https://reviews.llvm.org/D15. Should be less invasive for the va intrinsics as they're not used so widely. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:

[PATCH] D158246: [amdgpu] WIP variadics

JonChesterfield added inline comments. Comment at: llvm/lib/CodeGen/ExpandVAIntrinsics.cpp:159 +// conventions. Escape analysis on va_list values. +return false; + } Cut this from the WIP patch as the draft is noisy. Ran some tests through x64 with a pa

[PATCH] D158246: [amdgpu] WIP variadics

JonChesterfield added a comment. The lowering pass is broadly right, missing a few edge cases. Comment at: libc/config/gpu/entrypoints.txt:88 +libc.src.stdio.vsnprintf libc.src.stdio.puts libc.src.stdio.fopen ^these try to build, but fail. I haven

[PATCH] D158246: [amdgpu] WIP variadics

JonChesterfield created this revision. Herald added subscribers: libc-commits, foad, kerbowa, hiraditya, tpr, dstuttard, yaxunl, jvesely, kzhuravl, arsenm. Herald added projects: libc-project, All. JonChesterfield requested review of this revision. Herald added subscribers: llvm-commits, cfe-commi