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> &BV, Value *Fmt) {
- StringRef Str;
- if (!getConstantStringInfo(Fmt, Str) || Str.empty())
- return;
+static void locateCStrings(SparseBitVector<8> &BV, Value *Fmt, StringRef &Str)
{
----------------
Where is Fmt used in this function? Also, StringRef is passed by value. It's
already a lightweight reference, as demonstrated by its name.
================
Comment at: llvm/lib/Transforms/Utils/AMDGPUEmitPrintf.cpp:210
+// helper struct to package the string related data
+typedef struct StringData {
+ std::string Str = "";
----------------
typedef is not required for this in C++
================
Comment at: llvm/lib/Transforms/Utils/AMDGPUEmitPrintf.cpp:486-487
+ {ConstantInt::get(Int32Ty, 8)});
+ }
+ else {
+ // Include a dummy metadata instance in case of only non constant
----------------
else should be on the same line as the closing brace. Please do run
clang-format on the entire change.
================
Comment at: llvm/lib/Transforms/Utils/AMDGPUEmitPrintf.cpp:458
+ auto CreateControlDWord = M->getOrInsertFunction(
+ StringRef("__ockl_create_control_dword"), Builder.getInt32Ty(),
+ Builder.getInt32Ty(), Int1Ty, Int1Ty);
----------------
vikramRH wrote:
> arsenm wrote:
> > vikramRH wrote:
> > > arsenm wrote:
> > > > Do we really need another ockl control variable for this? Why isn't it
> > > > a parameter? printf=stdout always
> > > There are certain HIP API's such as "hip_assert" that output to stderr.
> > > currently such API's are supported via hostcalls. Although this
> > > implementation does not currently support the API's ,its kept as an
> > > option.
> > Right but the way to handle that would be a parameter for where to output,
> > not an externally set global
> I am not clear here, you expect additional inputs to device lib function ?
@arsenm, this "control word" written into the buffer. In that sense, it is
indeed a parameter passed from device to host as part of the printf packet. It
is not yet another global variable.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D150427/new/
https://reviews.llvm.org/D150427
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
