[PATCH] D141436: AMDGPU: Fix format string indexes for existing llvm.printf.fmts

[Matt Arsenault via Phabricator via llvm-commits]

arsenm created this revision.
arsenm added reviewers: AMDGPU, sameerds, vikramRH.
Herald added subscribers: kosarev, foad, kerbowa, arphaman, hiraditya, tpr, dstuttard, yaxunl, jvesely, kzhuravl.
Herald added a project: All.
arsenm requested review of this revision.
Herald added a subscriber: wdng.
Herald added a project: LLVM.

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.


https://reviews.llvm.org/D141436

Files:
  llvm/lib/Target/AMDGPU/AMDGPUPrintfRuntimeBinding.cpp
  llvm/test/CodeGen/AMDGPU/printf-existing-format-strings.ll


Index: llvm/test/CodeGen/AMDGPU/printf-existing-format-strings.ll
===================================================================
--- /dev/null
+++ llvm/test/CodeGen/AMDGPU/printf-existing-format-strings.ll
@@ -0,0 +1,52 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -mtriple=amdgcn-- -passes=amdgpu-printf-runtime-binding -mcpu=fiji -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
+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 2, 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 3, 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"}
Index: llvm/lib/Target/AMDGPU/AMDGPUPrintfRuntimeBinding.cpp
===================================================================
--- llvm/lib/Target/AMDGPU/AMDGPUPrintfRuntimeBinding.cpp
+++ llvm/lib/Target/AMDGPU/AMDGPUPrintfRuntimeBinding.cpp
@@ -161,7 +161,13 @@
   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() - 1;
 
   for (auto *CI : Printfs) {
     unsigned NumOps = CI->arg_size();
@@ -326,15 +332,6 @@
     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);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D141436.487994.patch
Type: text/x-patch
Size: 4574 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230110/86507cb8/attachment.bin>