[PATCH] D150427: [AMDGPU] Non hostcall printf support for HIP

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG631c965483e0: [AMDGPU] Non hostcall printf support for HIP (authored by vikramRH). Changed prior to commit:

[PATCH] D150427: [AMDGPU] Non hostcall printf support for HIP

arsenm accepted this revision. arsenm added a comment. This revision is now accepted and ready to land. lgtm with nit Comment at: llvm/lib/Transforms/Utils/AMDGPUEmitPrintf.cpp:280 +} else { + auto AllocSize = M->getDataLayout().getTypeAllocSize(Args[i]->getType()); +

[PATCH] D150427: [AMDGPU] Non hostcall printf support for HIP

vikramRH updated this revision to Diff 529865. vikramRH added a comment. Handled last set of review comments from @arsenm, would be willing to handle any new findings/concerns via additional patches Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D150427: [AMDGPU] Non hostcall printf support for HIP

arsenm added inline comments. Comment at: clang/test/CodeGenHIP/printf_nonhostcall.cpp:240 + return printf(s, 10); +} Test _BitInt for small and odd types, plus i128 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D150427: [AMDGPU] Non hostcall printf support for HIP

arsenm added inline comments. Comment at: clang/docs/ReleaseNotes.rst:589 + ``hostcall`` - printing happens during kernel execution via series of hostcalls, + The scheme requires the system to support pcie atomics.(default) + ``buffered`` - Scheme uses a debug buffer to

[PATCH] D150427: [AMDGPU] Non hostcall printf support for HIP

vikramRH updated this revision to Diff 529551. vikramRH added a comment. Reafactored non string arg handling into a seperate function with additional asserts Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150427/new/

[PATCH] D150427: [AMDGPU] Non hostcall printf support for HIP

sameerds added inline comments. Comment at: llvm/lib/Transforms/Utils/AMDGPUEmitPrintf.cpp:386-387 +} else { + auto IntTy = dyn_cast(Args[i]->getType()); + if (IntTy && IntTy->getBitWidth() == 32) +WhatToStore.push_back( vikramRH wrote: >

[PATCH] D150427: [AMDGPU] Non hostcall printf support for HIP

vikramRH added inline comments. Comment at: llvm/lib/Transforms/Utils/AMDGPUEmitPrintf.cpp:386-387 +} else { + auto IntTy = dyn_cast(Args[i]->getType()); + if (IntTy && IntTy->getBitWidth() == 32) +WhatToStore.push_back( arsenm wrote: >

[PATCH] D150427: [AMDGPU] Non hostcall printf support for HIP

arsenm added inline comments. Comment at: llvm/lib/Transforms/Utils/AMDGPUEmitPrintf.cpp:386-387 +} else { + auto IntTy = dyn_cast(Args[i]->getType()); + if (IntTy && IntTy->getBitWidth() == 32) +WhatToStore.push_back( vikramRH wrote: >

[PATCH] D150427: [AMDGPU] Non hostcall printf support for HIP

vikramRH updated this revision to Diff 529219. vikramRH marked an inline comment as done. vikramRH added a comment. updated formating Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150427/new/ https://reviews.llvm.org/D150427 Files:

[PATCH] D150427: [AMDGPU] Non hostcall printf support for HIP

vikramRH marked 4 inline comments as done. vikramRH added inline comments. Comment at: clang/test/CodeGenHIP/printf_nonhostcall.cpp:137 + +__device__ float f1 = 3.14f; +__device__ double f2 = 2.71828; arsenm wrote: > Also half C++ default arg promotions does

[PATCH] D150427: [AMDGPU] Non hostcall printf support for HIP

vikramRH updated this revision to Diff 529207. vikramRH added a comment. Further review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150427/new/ https://reviews.llvm.org/D150427 Files: clang/docs/ReleaseNotes.rst

[PATCH] D150427: [AMDGPU] Non hostcall printf support for HIP

arsenm added a comment. Also should be noted in the release notes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150427/new/ https://reviews.llvm.org/D150427 ___ cfe-commits mailing list

[PATCH] D150427: [AMDGPU] Non hostcall printf support for HIP

arsenm added inline comments. Comment at: clang/test/CodeGenHIP/printf_nonhostcall.cpp:137 + +__device__ float f1 = 3.14f; +__device__ double f2 = 2.71828; Also half Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D150427: [AMDGPU] Non hostcall printf support for HIP

arsenm added inline comments. Comment at: llvm/lib/Transforms/Utils/AMDGPUEmitPrintf.cpp:371 +// the offsets. +uint64_t DstAlign = (i == 0) ? 4 : 8; +Builder.CreateMemCpy(PtrToStore, /*DstAlign*/ Align(DstAlign), Args[i], I don't follow

[PATCH] D150427: [AMDGPU] Non hostcall printf support for HIP

vikramRH marked 4 inline comments as done. vikramRH added inline comments. Comment at: clang/test/CodeGenHIP/printf_nonhostcall.cpp:228 + return printf(s, 10); +} arsenm wrote: > vikramRH wrote: > > arsenm wrote: > > > Need some vector tests, especially 3 x

[PATCH] D150427: [AMDGPU] Non hostcall printf support for HIP

vikramRH updated this revision to Diff 528407. vikramRH added a comment. Handled few more review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150427/new/ https://reviews.llvm.org/D150427 Files:

[PATCH] D150427: [AMDGPU] Non hostcall printf support for HIP

arsenm added a comment. Should also be mentioned in the release notes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150427/new/ https://reviews.llvm.org/D150427 ___ cfe-commits mailing list

[PATCH] D150427: [AMDGPU] Non hostcall printf support for HIP

arsenm added inline comments. Comment at: clang/include/clang/Driver/Options.td:1030 NegFlag>; +def mprintf_kind_EQ : Joined<["-"], "mprintf-kind=">, Group, + HelpText<"Specify the printf lowering scheme (AMDGPU only), allowed values are " vikramRH wrote: >

[PATCH] D150427: [AMDGPU] Non hostcall printf support for HIP

vikramRH updated this revision to Diff 527472. vikramRH added a comment. new line at the end of test file Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150427/new/ https://reviews.llvm.org/D150427 Files:

[PATCH] D150427: [AMDGPU] Non hostcall printf support for HIP

vikramRH updated this revision to Diff 527462. vikramRH added a comment. Herald added a subscriber: jdoerfert. Few additional changes, 1. reflect printf-kind in module flags metadata 2. Test cases for the change Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D150427: [AMDGPU] Non hostcall printf support for HIP

vikramRH marked 2 inline comments as done. vikramRH added inline comments. Comment at: clang/include/clang/Driver/Options.td:1030 NegFlag>; +def mprintf_kind_EQ : Joined<["-"], "mprintf-kind=">, Group, + HelpText<"Specify the printf lowering scheme (AMDGPU only), allowed

[PATCH] D150427: [AMDGPU] Non hostcall printf support for HIP

vikramRH marked 11 inline comments as done. vikramRH added inline comments. Comment at: clang/test/CodeGenHIP/printf_nonhostcall.cpp:64 +// CHECK-NEXT:[[PRINTBUFFNEXTPTR4:%.*]] = getelementptr inbounds i8, ptr addrspace(1) [[PRINTBUFFNEXTPTR3]], i64 [[TMP13]] +//

[PATCH] D150427: [AMDGPU] Non hostcall printf support for HIP

vikramRH updated this revision to Diff 526963. vikramRH added a comment. Handled most review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150427/new/ https://reviews.llvm.org/D150427 Files: clang/include/clang/Basic/TargetOptions.h

[PATCH] D150427: [AMDGPU] Non hostcall printf support for HIP

arsenm added inline comments. Comment at: llvm/lib/Transforms/Utils/AMDGPUEmitPrintf.cpp:211 +struct StringData { + std::string Str = ""; + bool isConst = true; arsenm wrote: > Don't need = "" Can't you just use the raw StringRef out of getConstantStringInfo

[PATCH] D150427: [AMDGPU] Non hostcall printf support for HIP

arsenm added inline comments. Comment at: clang/include/clang/Driver/Options.td:1030 NegFlag>; +def mprintf_kind_EQ : Joined<["-"], "mprintf-kind=">, Group, + HelpText<"Specify the printf lowering scheme (AMDGPU only), allowed values are " I'm a bit worried

[PATCH] D150427: [AMDGPU] Non hostcall printf support for HIP

vikramRH updated this revision to Diff 525946. vikramRH added a comment. rebase and ping, apologies for the short frequency. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150427/new/ https://reviews.llvm.org/D150427 Files:

[PATCH] D150427: [AMDGPU] Non hostcall printf support for HIP

vikramRH updated this revision to Diff 525082. vikramRH added a comment. Handled review comments and rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150427/new/ https://reviews.llvm.org/D150427 Files:

[PATCH] D150427: [AMDGPU] Non hostcall printf support for HIP

arsenm added inline comments. Comment at: clang/test/CodeGenHIP/printf_nonhostcall.cpp:170-171 +// CHECK-NEXT:[[PRINTBUFFNEXTPTR5:%.*]] = getelementptr i8, ptr addrspace(1) [[PRINTBUFFNEXTPTR4]], i32 8 +// CHECK-NEXT:[[TMP13:%.*]] = bitcast double [[TMP4]] to i64 +//

[PATCH] D150427: [AMDGPU] Non hostcall printf support for HIP

arsenm added inline comments. Comment at: llvm/lib/Transforms/Utils/AMDGPUEmitPrintf.cpp:219 + +static inline size_t alignUp(size_t Value, uint Alignment) { + return (Value + Alignment - 1) & ~(Alignment - 1); MathExtras already has alignTo

[PATCH] D150427: [AMDGPU] Non hostcall printf support for HIP

vikramRH updated this revision to Diff 524590. vikramRH added a comment. Handled review comments and rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150427/new/ https://reviews.llvm.org/D150427 Files:

[PATCH] D150427: [AMDGPU] Non hostcall printf support for HIP

sameerds added inline comments. Comment at: llvm/lib/Transforms/Utils/AMDGPUEmitPrintf.cpp:184 // specify a string, i.e, the "%s" specifier with optional '*' characters. -static void locateCStrings(SparseBitVector<8> , Value *Fmt) { - StringRef Str; - if

[PATCH] D150427: [AMDGPU] Non hostcall printf support for HIP

vikramRH added inline comments. Comment at: llvm/lib/Transforms/Utils/AMDGPUEmitPrintf.cpp:458 +auto CreateControlDWord = M->getOrInsertFunction( +StringRef("__ockl_create_control_dword"), Builder.getInt32Ty(), +Builder.getInt32Ty(), Int1Ty, Int1Ty);

[PATCH] D150427: [AMDGPU] Non hostcall printf support for HIP

vikramRH added inline comments. Comment at: llvm/lib/Transforms/Utils/AMDGPUEmitPrintf.cpp:426-427 + +// The buffered version still follows OpenCL printf standards for +// printf return value, i.e 0 on success, 1 on failure. +ConstantPointerNull *zeroIntPtr =

[PATCH] D150427: [AMDGPU] Non hostcall printf support for HIP

vikramRH updated this revision to Diff 523344. vikramRH added a comment. Hanldles Review comments from @sameerds Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150427/new/ https://reviews.llvm.org/D150427 Files:

[PATCH] D150427: [AMDGPU] Non hostcall printf support for HIP

sameerds added inline comments. Comment at: llvm/lib/Transforms/Utils/AMDGPUEmitPrintf.cpp:184-185 // specify a string, i.e, the "%s" specifier with optional '*' characters. -static void locateCStrings(SparseBitVector<8> , Value *Fmt) { - StringRef Str; - if

[PATCH] D150427: [AMDGPU] Non hostcall printf support for HIP

arsenm added inline comments. Comment at: clang/include/clang/Basic/TargetOptions.h:99 + +/// pritnf lowering scheme involving implicit printf buffers, +Buffered = 1, Typo pritnf Comment at:

[PATCH] D150427: [AMDGPU] Non hostcall printf support for HIP

sameerds added inline comments. Comment at: llvm/lib/Transforms/Utils/AMDGPUEmitPrintf.cpp:212 +// helper struct to package the string related data +typedef struct S { + std::string Str; you can just say "struct StringData" Comment at:

[PATCH] D150427: [AMDGPU] Non hostcall printf support for HIP

vikramRH added inline comments. Comment at: llvm/lib/Transforms/Utils/AMDGPUEmitPrintf.cpp:458 +auto CreateControlDWord = M->getOrInsertFunction( +StringRef("__ockl_create_control_dword"), Builder.getInt32Ty(), +Builder.getInt32Ty(), Int1Ty, Int1Ty);

[PATCH] D150427: [AMDGPU] Non hostcall printf support for HIP

vikramRH added a comment. In D150427#4338334 , @jhuber6 wrote: > Where does the runtime implementation of this live? I'm not very familiar > with the HIP / hostcall ecosystem. Currently there is a rocclr review up internally for the runtime side

[PATCH] D150427: [AMDGPU] Non hostcall printf support for HIP

vikramRH updated this revision to Diff 522954. vikramRH added a comment. 1. Updating D150427 : [AMDGPU] Non hostcall printf support for HIP # 2. Enter a brief description of the changes included in this update. 3. The first line is used as subject, next lines

[PATCH] D150427: [AMDGPU] Non hostcall printf support for HIP

arsenm added inline comments. Comment at: clang/include/clang/Basic/LangOptions.def:274 LANGOPT(OffloadingNewDriver, 1, 0, "use the new driver for generating offloading code.") +ENUM_LANGOPT(AMDGPUPrintfKindVal, AMDGPUPrintfKind, 2, AMDGPUPrintfKind::Buffered, "printf

[PATCH] D150427: [AMDGPU] Non hostcall printf support for HIP

jhuber6 added a comment. Where does the runtime implementation of this live? I'm not very familiar with the HIP / hostcall ecosystem. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150427/new/ https://reviews.llvm.org/D150427

[PATCH] D150427: [AMDGPU] Non hostcall printf support for HIP

yaxunl added inline comments. Comment at: clang/include/clang/Driver/Options.td:1036 + NormalizedValues<["Hostcall", "Buffered"]>, + MarshallingInfoEnum, "Hostcall">; def fgpu_default_stream_EQ : Joined<["-"], "fgpu-default-stream=">, as a target option,

[PATCH] D150427: [AMDGPU] Non hostcall printf support for HIP

yaxunl added inline comments. Comment at: clang/include/clang/Basic/LangOptions.def:274 LANGOPT(OffloadingNewDriver, 1, 0, "use the new driver for generating offloading code.") +ENUM_LANGOPT(AMDGPUPrintfKindVal, AMDGPUPrintfKind, 2, AMDGPUPrintfKind::Buffered, "printf

[PATCH] D150427: [AMDGPU] Non hostcall printf support for HIP

vikramRH created this revision. vikramRH added reviewers: sameerds, b-sumner, yaxunl, arsenm. Herald added subscribers: hoy, kerbowa, hiraditya, Anastasia, tpr, dstuttard, jvesely, kzhuravl. Herald added a project: All. vikramRH requested review of this revision. Herald added subscribers: