[PATCH] D93454: [LLVM-C] Replace LLVMSetInstDebugLocation with LLVMAddMetadataToInst.

[Florian Hahn via Phabricator via llvm-commits]

fhahn updated this revision to Diff 373242.
fhahn added a comment.



In D93454#3006357 <https://reviews.llvm.org/D93454#3006357>, @tstellar wrote:

> In D93454#3005834 <https://reviews.llvm.org/D93454#3005834>, @fhahn wrote:
>
>> In D93454#3001992 <https://reviews.llvm.org/D93454#3001992>, @tstellar wrote:
>>
>>> This is an API break to the C API, which is allowed, but is there any way we can keep the old function?
>>
>> We could keep the old function and just have it call `AddMetadataToInst`. There should be no functional change *if* the user does not use the new functionality to collect non-debug metadata in the builder. Not sure if that's a great option. We could also extend `AddMetadataToInst` to accept a filter to only copy specific metadata kinds and have `LLVMAddMetadataToInst` use this filter. But this would also mean that we need to maintain a change `IRBuilder` which is only there for C API compatibility. In that case, we might as well keep the old `SetInstDebugLocation`. What do you think?
>
> To me it sounds like keeping the old function would be best.  We can also deprecate it in the release notes so that users are aware that it might be removed at some point, and that it might not work with the new functionality.

Sounds good, I updated the patch to only add `LLVMAddMetadataToInst` and deprecate `LLVMAddMetadataToInst`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93454

Files:
  llvm/docs/ReleaseNotes.rst
  llvm/include/llvm-c/Core.h
  llvm/lib/IR/Core.cpp
  llvm/tools/llvm-c-test/echo.cpp


Index: llvm/tools/llvm-c-test/echo.cpp
===================================================================
--- llvm/tools/llvm-c-test/echo.cpp
+++ llvm/tools/llvm-c-test/echo.cpp
@@ -855,7 +855,7 @@
       LLVMSetMetadata(Dst, Kind, LLVMMetadataAsValue(Ctx, MD));
     }
     LLVMDisposeValueMetadataEntries(AllMetadata);
-    LLVMSetInstDebugLocation(Builder, Dst);
+    LLVMAddMetadataToInst(Builder, Dst);
 
     check_value_kind(Dst, LLVMInstructionValueKind);
     return VMap[Src] = Dst;
Index: llvm/lib/IR/Core.cpp
===================================================================
--- llvm/lib/IR/Core.cpp
+++ llvm/lib/IR/Core.cpp
@@ -3133,6 +3133,10 @@
   unwrap(Builder)->SetInstDebugLocation(unwrap<Instruction>(Inst));
 }
 
+void LLVMAddMetadataToInst(LLVMBuilderRef Builder, LLVMValueRef Inst) {
+  unwrap(Builder)->AddMetadataToInst(unwrap<Instruction>(Inst));
+}
+
 void LLVMBuilderSetDefaultFPMathTag(LLVMBuilderRef Builder,
                                     LLVMMetadataRef FPMathTag) {
 
Index: llvm/include/llvm-c/Core.h
===================================================================
--- llvm/include/llvm-c/Core.h
+++ llvm/include/llvm-c/Core.h
@@ -3611,10 +3611,20 @@
  * current debug location for the given builder.  If the builder has no current
  * debug location, this function is a no-op.
  *
+ * LLVMSetInstDebugLocation is deprecated in favor of the more general
+ * LLVMAddMetadataToInst.
+ *
  * @see llvm::IRBuilder::SetInstDebugLocation()
  */
 void LLVMSetInstDebugLocation(LLVMBuilderRef Builder, LLVMValueRef Inst);
 
+/**
+ * Adds the metadata registered with the given builder to the given instruction.
+ *
+ * @see llvm::IRBuilder::AddMetadataToInst()
+ */
+void LLVMAddMetadataToInst(LLVMBuilderRef Builder, LLVMValueRef Inst);
+
 /**
  * Get the dafult floating-point math metadata for a given builder.
  *
Index: llvm/docs/ReleaseNotes.rst
===================================================================
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -123,7 +123,8 @@
 Changes to the C API
 --------------------
 
-* ...
+* ``LLVMSetInstDebugLocation`` has been deprecated in favor of the more general
+  ``LLVMAddMetadataToInst``.
 
 Changes to the Go bindings
 --------------------------


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D93454.373242.patch
Type: text/x-patch
Size: 2263 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210917/a0e8d908/attachment.bin>