[PATCH] D71365: expand printf when compiling HIP to AMDGPU

This revision was automatically updated to reflect the committed changes. Closed by commit rGed181efa175d: [HIP][AMDGPU] expand printf when compiling HIP to AMDGPU (authored by sameerds). Herald added a subscriber: kerbowa. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D71365: expand printf when compiling HIP to AMDGPU

arsenm accepted this revision. arsenm added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71365/new/ https://reviews.llvm.org/D71365

[PATCH] D71365: expand printf when compiling HIP to AMDGPU

sameerds marked 2 inline comments as done. sameerds added inline comments. Comment at: clang/test/CodeGenHIP/printf.cpp:18 +} + +// CHECK: [[BEGIN:%.*]] = call i64 @__ockl_printf_begin(i64 0) arsenm wrote: > sameerds wrote: > > sameerds wrote: > > > arsenm

[PATCH] D71365: expand printf when compiling HIP to AMDGPU

sameerds updated this revision to Diff 237864. sameerds added a comment. - Added a test for address spaces - Removed an unnecessary addrspacecast in the strlen expansion - Updated the tests to pass -fcuda-is-device - printf is not a builtin for device code, so removed the code and tests related

[PATCH] D71365: expand printf when compiling HIP to AMDGPU

arsenm added a comment. Mostly looks fine, except vectors are supposed to work Comment at: llvm/lib/Transforms/Utils/AMDGPUEmitPrintf.cpp:52 + } else if (Ty->getTypeID() == Type::DoubleTyID) { +return Builder.CreateBitCast(Arg, Int64Ty); + } else if (auto PtrTy =

[PATCH] D71365: expand printf when compiling HIP to AMDGPU

arsenm added inline comments. Comment at: clang/test/CodeGenHIP/printf.cpp:18 +} + +// CHECK: [[BEGIN:%.*]] = call i64 @__ockl_printf_begin(i64 0) sameerds wrote: > sameerds wrote: > > arsenm wrote: > > > This could use a lot more testcases. Can you add some

[PATCH] D71365: expand printf when compiling HIP to AMDGPU

sameerds added inline comments. Comment at: clang/test/CodeGenHIP/printf.cpp:18 +} + +// CHECK: [[BEGIN:%.*]] = call i64 @__ockl_printf_begin(i64 0) sameerds wrote: > arsenm wrote: > > This could use a lot more testcases. Can you add some half, float, and > >

[PATCH] D71365: expand printf when compiling HIP to AMDGPU

sameerds added a comment. In D71365#1807872 , @b-sumner wrote: > Should this be looking forward to also handling OpenCL, which does require > vector support? Sure. The implementation is general enough that we can point at the two places in

[PATCH] D71365: expand printf when compiling HIP to AMDGPU

b-sumner added a comment. Should this be looking forward to also handling OpenCL, which does require vector support? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71365/new/ https://reviews.llvm.org/D71365

[PATCH] D71365: expand printf when compiling HIP to AMDGPU

sameerds added inline comments. Comment at: clang/test/CodeGenHIP/printf.cpp:18 +} + +// CHECK: [[BEGIN:%.*]] = call i64 @__ockl_printf_begin(i64 0) arsenm wrote: > This could use a lot more testcases. Can you add some half, float, and double > as well as

[PATCH] D71365: expand printf when compiling HIP to AMDGPU

arsenm added inline comments. Comment at: clang/test/CodeGenHIP/printf.cpp:18 +} + +// CHECK: [[BEGIN:%.*]] = call i64 @__ockl_printf_begin(i64 0) This could use a lot more testcases. Can you add some half, float, and double as well as pointers (including

[PATCH] D71365: expand printf when compiling HIP to AMDGPU

sameerds updated this revision to Diff 236312. sameerds edited the summary of this revision. sameerds added a comment. Improved the test defined in clang/test/CodeGenHIP/printf.cpp Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71365/new/

[PATCH] D71365: expand printf when compiling HIP to AMDGPU

sameerds created this revision. Herald added subscribers: llvm-commits, cfe-commits, hiraditya, t-tye, tpr, dstuttard, yaxunl, mgorny, nhaehnle, wdng, jvesely, kzhuravl. Herald added projects: clang, LLVM. This change implements the expansion in two parts: - Add a utility function