[PATCH] D150427: [AMDGPU] Non hostcall printf support for HIP
Matt Arsenault via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Jun 5 16:17:46 PDT 2023
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 this alignment logic, but I guess the optimizer will figure it out later anyway so I suppose the Align(1) was fine.
Also should use Align.
================
Comment at: llvm/lib/Transforms/Utils/AMDGPUEmitPrintf.cpp:373
+ Builder.CreateMemCpy(PtrToStore, /*DstAlign*/ Align(DstAlign), Args[i],
+ /*SrcAlign*/ Align(1), StrIt->RealSize);
+
----------------
The source align is known
================
Comment at: llvm/lib/Transforms/Utils/AMDGPUEmitPrintf.cpp:386-387
+ } else {
+ auto IntTy = dyn_cast<IntegerType>(Args[i]->getType());
+ if (IntTy && IntTy->getBitWidth() == 32)
+ WhatToStore.push_back(
----------------
isIntegerTy(32).
I also do not understand why only 32-bit integers would be promoted to 64-bit (or why this would be zext). This doesn't match varargs ABI handling, where everything smaller would be promoted to i32.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D150427/new/
https://reviews.llvm.org/D150427
More information about the llvm-commits
mailing list