[llvm] 39af5ce - AMDGPU: Fix format string indexes for existing llvm.printf.fmts

[Matt Arsenault via llvm-commits]

Author: Matt Arsenault
Date: 2023-01-13T13:18:27-05:00
New Revision: 39af5cec8b066901b374ae52179cd378dceb2e0e

URL: https://github.com/llvm/llvm-project/commit/39af5cec8b066901b374ae52179cd378dceb2e0e
DIFF: https://github.com/llvm/llvm-project/commit/39af5cec8b066901b374ae52179cd378dceb2e0e.diff

LOG: AMDGPU: Fix format string indexes for existing llvm.printf.fmts

The index stored to the buffer is just an index into this named
metadata. It would more robust to produce a private constant table,
and use a constant expression to index into it.

Added: 
    llvm/test/CodeGen/AMDGPU/printf-existing-format-strings.ll

Modified: 
    llvm/lib/Target/AMDGPU/AMDGPUPrintfRuntimeBinding.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Target/AMDGPU/AMDGPUPrintfRuntimeBinding.cpp b/llvm/lib/Target/AMDGPU/AMDGPUPrintfRuntimeBinding.cpp
index 7018f0a8c2a5..50075ff0bf8c 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUPrintfRuntimeBinding.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUPrintfRuntimeBinding.cpp
@@ -161,7 +161,13 @@ bool AMDGPUPrintfRuntimeBindingImpl::lowerPrintfForGpu(Module &M) {
   LLVMContext &Ctx = M.getContext();
   IRBuilder<> Builder(Ctx);
   Type *I32Ty = Type::getInt32Ty(Ctx);
-  unsigned UniqID = 0;
+
+  // Instead of creating global variables, the printf format strings are
+  // extracted and passed as metadata. This avoids polluting llvm's symbol
+  // tables in this module. Metadata is going to be extracted by the backend
+  // passes and inserted into the OpenCL binary as appropriate.
+  NamedMDNode *metaD = M.getOrInsertNamedMetadata("llvm.printf.fmts");
+  unsigned UniqID = metaD->getNumOperands();
 
   for (auto *CI : Printfs) {
     unsigned NumOps = CI->arg_size();
@@ -326,15 +332,6 @@ bool AMDGPUPrintfRuntimeBindingImpl::lowerPrintfForGpu(Module &M) {
     std::string fmtstr = itostr(++UniqID) + ":" + Sizes.str();
     MDString *fmtStrArray = MDString::get(Ctx, fmtstr);
 
-    // Instead of creating global variables, the
-    // printf format strings are extracted
-    // and passed as metadata. This avoids
-    // polluting llvm's symbol tables in this module.
-    // Metadata is going to be extracted
-    // by the backend passes and inserted
-    // into the OpenCL binary as appropriate.
-    StringRef amd("llvm.printf.fmts");
-    NamedMDNode *metaD = M.getOrInsertNamedMetadata(amd);
     MDNode *myMD = MDNode::get(Ctx, fmtStrArray);
     metaD->addOperand(myMD);
     Value *sumC = ConstantInt::get(SizetTy, Sum, false);

diff  --git a/llvm/test/CodeGen/AMDGPU/printf-existing-format-strings.ll b/llvm/test/CodeGen/AMDGPU/printf-existing-format-strings.ll
new file mode 100644
index 000000000000..8a3e002cb57c
--- /dev/null
+++ b/llvm/test/CodeGen/AMDGPU/printf-existing-format-strings.ll
@@ -0,0 +1,63 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --check-globals
+; RUN: opt -mtriple=amdgcn-- -passes=amdgpu-printf-runtime-binding -S < %s | FileCheck --check-prefix=GCN %s
+
+; Make sure the correct index is used for newly added entries to
+; llvm.printf.fmts if one already exists in the module.
+
+ at format.str.one.int = private unnamed_addr addrspace(4) constant [8 x i8] c"arst %d\00", align 1
+
+; Inserted IDs should start at 2
+;.
+; GCN: @[[FORMAT_STR_ONE_INT:[a-zA-Z0-9_$"\\.-]+]] = private unnamed_addr addrspace(4) constant [8 x i8] c"arst %d\00", align 1
+;.
+define void @call_printf(i32 %n, i32 %m) {
+; GCN-LABEL: @call_printf(
+; GCN-NEXT:    [[PRINTF_ALLOC_FN:%.*]] = call ptr addrspace(1) @__printf_alloc(i32 8)
+; GCN-NEXT:    br label [[DOTSPLIT:%.*]]
+; GCN:       .split:
+; GCN-NEXT:    [[TMP1:%.*]] = icmp ne ptr addrspace(1) [[PRINTF_ALLOC_FN]], null
+; GCN-NEXT:    br i1 [[TMP1]], label [[TMP2:%.*]], label [[TMP3:%.*]]
+; GCN:       2:
+; GCN-NEXT:    [[PRINTBUFFID:%.*]] = getelementptr i8, ptr addrspace(1) [[PRINTF_ALLOC_FN]], i32 0
+; GCN-NEXT:    [[PRINTBUFFIDCAST:%.*]] = bitcast ptr addrspace(1) [[PRINTBUFFID]] to ptr addrspace(1)
+; GCN-NEXT:    store i32 3, ptr addrspace(1) [[PRINTBUFFIDCAST]], align 4
+; GCN-NEXT:    [[PRINTBUFFGEP:%.*]] = getelementptr i8, ptr addrspace(1) [[PRINTF_ALLOC_FN]], i32 4
+; GCN-NEXT:    [[PRINTBUFFPTRCAST:%.*]] = bitcast ptr addrspace(1) [[PRINTBUFFGEP]] to ptr addrspace(1)
+; GCN-NEXT:    store i32 [[N:%.*]], ptr addrspace(1) [[PRINTBUFFPTRCAST]], align 4
+; GCN-NEXT:    br label [[TMP3]]
+; GCN:       3:
+; GCN-NEXT:    [[PRINTF_ALLOC_FN1:%.*]] = call ptr addrspace(1) @__printf_alloc(i32 8)
+; GCN-NEXT:    br label [[DOTSPLIT2:%.*]]
+; GCN:       .split2:
+; GCN-NEXT:    [[TMP4:%.*]] = icmp ne ptr addrspace(1) [[PRINTF_ALLOC_FN1]], null
+; GCN-NEXT:    br i1 [[TMP4]], label [[TMP5:%.*]], label [[TMP6:%.*]]
+; GCN:       5:
+; GCN-NEXT:    [[PRINTBUFFID3:%.*]] = getelementptr i8, ptr addrspace(1) [[PRINTF_ALLOC_FN1]], i32 0
+; GCN-NEXT:    [[PRINTBUFFIDCAST4:%.*]] = bitcast ptr addrspace(1) [[PRINTBUFFID3]] to ptr addrspace(1)
+; GCN-NEXT:    store i32 4, ptr addrspace(1) [[PRINTBUFFIDCAST4]], align 4
+; GCN-NEXT:    [[PRINTBUFFGEP5:%.*]] = getelementptr i8, ptr addrspace(1) [[PRINTF_ALLOC_FN1]], i32 4
+; GCN-NEXT:    [[PRINTBUFFPTRCAST6:%.*]] = bitcast ptr addrspace(1) [[PRINTBUFFGEP5]] to ptr addrspace(1)
+; GCN-NEXT:    store i32 [[M:%.*]], ptr addrspace(1) [[PRINTBUFFPTRCAST6]], align 4
+; GCN-NEXT:    br label [[TMP6]]
+; GCN:       6:
+; GCN-NEXT:    ret void
+;
+  %call0 = call i32 @printf(ptr addrspace(4) @format.str.one.int, i32 %n)
+  %call1 = call i32 @printf(ptr addrspace(4) @format.str.one.int, i32 %m)
+  ret void
+}
+
+declare i32 @printf(ptr addrspace(4), ...)
+
+!llvm.printf.fmts = !{!0, !1}
+
+!0 = !{!"49:2:4:4:%s\\72%d"}
+!1 = !{!"50:1:8:arst %p"}
+;.
+; GCN: attributes #[[ATTR0:[0-9]+]] = { nounwind }
+;.
+; GCN: [[META0:![0-9]+]] = !{!"49:2:4:4:%s\\72%d"}
+; GCN: [[META1:![0-9]+]] = !{!"50:1:8:arst %p"}
+; GCN: [[META2:![0-9]+]] = !{!"3:1:4:arst %d"}
+; GCN: [[META3:![0-9]+]] = !{!"4:1:4:arst %d"}
+;.