[PATCH] D53911: [Orc] make getResponsibilitySetWithLegacyFn behavior match with LegacyJITSymbolResolver::getResponsibilitySet

[Taewook Oh via Phabricator via llvm-commits]

twoh created this revision.
twoh added a reviewer: lhames.
Herald added a subscriber: llvm-commits.

https://reviews.llvm.org/rL340874 replaces `lookupFlags` with `getResponsibilitySet`, and this introduces a behaviral change in `LegacyJITSymbolResolver::getResponsibilitySet` (If there is no existing definition then the caller is responsible for materializing it. https://github.com/llvm-mirror/llvm/blob/master/lib/ExecutionEngine/RuntimeDyld/JITSymbol.cpp#L127). However, this changes is missed from `getResponsibilitySetWithLegacyFn`. Though I'm not very familiar with Orc code base, I think the behavior of these two functions supposed to match, and the patch actually fixes "symbols not found" error in our codebase using OrcJIT.


Repository:
  rL LLVM

https://reviews.llvm.org/D53911

Files:
  include/llvm/ExecutionEngine/Orc/Legacy.h
  unittests/ExecutionEngine/Orc/LegacyAPIInteropTest.cpp


Index: unittests/ExecutionEngine/Orc/LegacyAPIInteropTest.cpp
===================================================================
--- unittests/ExecutionEngine/Orc/LegacyAPIInteropTest.cpp
+++ unittests/ExecutionEngine/Orc/LegacyAPIInteropTest.cpp
@@ -94,7 +94,7 @@
       getResponsibilitySetWithLegacyFn(SymbolNameSet({Bar, Baz}), LegacyLookup);
 
   EXPECT_TRUE(!!RS) << "Expected getResponsibilitySetWithLegacyFn to succeed";
-  EXPECT_EQ(RS->size(), 1U) << "Wrong number of symbols returned";
+  EXPECT_EQ(RS->size(), 2U) << "Wrong number of symbols returned";
   EXPECT_EQ(RS->count(Bar), 1U) << "Incorrect responsibility set returned";
   EXPECT_FALSE(BarMaterialized)
       << "lookupFlags should not have materialized bar";
Index: include/llvm/ExecutionEngine/Orc/Legacy.h
===================================================================
--- include/llvm/ExecutionEngine/Orc/Legacy.h
+++ include/llvm/ExecutionEngine/Orc/Legacy.h
@@ -125,6 +125,11 @@
         Result.insert(S);
     } else if (auto Err = Sym.takeError())
       return std::move(Err);
+    else {
+      // If there is no existing definition then the caller is responsible for
+      // it.
+      Result.insert(S);
+    }
   }
 
   return Result;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D53911.171836.patch
Type: text/x-patch
Size: 1228 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20181030/fee3de65/attachment.bin>