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 populate printf varargs, does
not
----------------
I thought the default was buffered, such that it would always work. Defaults
that always work are better
Also, amdgpu-arch should probably learn to report if pcie atomics work to
improve the autodetect
================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4705-4707
+ // Force compiler error on invalid conversion specifiers
+ CmdArgs.push_back(
+ Args.MakeArgString("-Werror=format-invalid-specifier"));
----------------
I don't see why we would special case this
================
Comment at: llvm/lib/Transforms/Utils/AMDGPUEmitPrintf.cpp:361
+ default:
+ llvm_unreachable("unexpected Integer Type");
+ }
----------------
It costs nothing to handle all types < 64. For larger just return the original
value
================
Comment at: llvm/lib/Transforms/Utils/AMDGPUEmitPrintf.cpp:368
+
+ if (Ty->isHalfTy())
+ return Builder.CreateFPExt(Arg, Builder.getDoubleTy());
----------------
Should avoid special casing this too, there's also bfloat to consider at a
minimum
================
Comment at: llvm/lib/Transforms/Utils/AMDGPUEmitPrintf.cpp:371
+
+ llvm_unreachable("unexpected type");
+}
----------------
Safest to just return the original value
================
Comment at: llvm/lib/Transforms/Utils/AMDGPUEmitPrintf.cpp:413
+
+ for (unsigned I = 0, E = WhatToStore.size(); I != E; ++I) {
+ Value *toStore = WhatToStore[I];
----------------
Range loop, index is unused
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
