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
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
