[PATCH] D141173: AMDGPU: Use getConstantStringInfo for printf format strings

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/test/CodeGen/AMDGPU/opencl-printf.ll:549
+entry:
+  %call1 = call i32 (ptr addrspace(4), ...) @printf(ptr addrspace(4) @format.str.no.null.terminator, i32 %n)
+  ret void
----------------
sameerds wrote:
> I can't make out from the spec if OpenCL allows a format string with no terminator. Is this UB? Is it supposed to produce a diagnostic?
If you have a string literal in C it ends up being a null terminator. This covers not crashing on valid IR that opencl just happens to never produce


================
Comment at: llvm/test/CodeGen/AMDGPU/opencl-printf.ll:605
+entry:
+  %call1 = call i32 (ptr addrspace(4), ...) @printf(ptr addrspace(4) getelementptr ([11 x i8], ptr addrspace(4) @indexed.format.str, i64 0, i64 11), i32 %n)
+  ret void
----------------
sameerds wrote:
> Same question. UB? Diagnostic? 
We can handle it just fine, no need to diagnose


================
Comment at: llvm/test/CodeGen/AMDGPU/opencl-printf.ll:625
 ; GCN-NEXT:    [[PRINTBUFFIDCAST:%.*]] = bitcast ptr addrspace(1) [[PRINTBUFFID]] to ptr addrspace(1)
-; GCN-NEXT:    store i32 12, ptr addrspace(1) [[PRINTBUFFIDCAST]], align 4
+; GCN-NEXT:    store i32 15, ptr addrspace(1) [[PRINTBUFFIDCAST]], align 4
 ; GCN-NEXT:    [[PRINTBUFFGEP:%.*]] = getelementptr i8, ptr addrspace(1) [[PRINTF_ALLOC_FN]], i32 4
----------------
sameerds wrote:
> Are the previous values plain wrong? Can this be reproduced in an actual OpenCL kernel? Will it crash the kernel?
These are just the indexes into the list of format string metadata, these will get renumbered every time a new function is inserted into this file


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141173/new/

https://reviews.llvm.org/D141173