[PATCH] D19373: Remangle intrinsics names when types are renamed

Artur Pilipenko via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 25 05:23:41 PDT 2016


apilipenko added inline comments.

================
Comment at: lib/AsmParser/LLParser.cpp:223
@@ +222,3 @@
+        if (CallInst *CI = dyn_cast<CallInst>(U))
+          CI->setCalledFunction(Remangled.getValue());
+      F->eraseFromParent();
----------------
joker.eph wrote:
> RAUW does not work at this point?
It works, autoupgrade uses it, but what are the benefits over changing the existing call instruction?

================
Comment at: lib/IR/Function.cpp:1079
@@ -1049,1 +1078,3 @@
+}
+
 /// hasAddressTaken - returns true if there are any uses of this function
----------------
joker.eph wrote:
> There are two conceptually separated path IIUC:
> 
> 1) old bitcode the intrinsic is *not* mangled and need to be mangled. This is the auto-upgrade path.
> 2) current bitcode has named type collision in the context and you need to remangle.
> 
> The fact that the remangling is shared is fine. However the conditions to reach this should be asserted better: the first path is valid only if the intrinsic was not mangled in the first place. The second path is valid only if the mismatch is because of a suffix added to the type.
In the first "not mangled intrinsic" path do you mean masked load/store intrinsics specifically?

In general, first I try to upgrade an intrinsic (see BitcodeReader.cpp:3220). If it was upgraded I assume that the name and the signature matches and no further remangling is needed. If it wasn't upgraded I check if it needs remangling. We can only remangle intrinsics which signature matches with the .td descriptor, that's why I do upgrade first.

With masked load/store autoupgrade patch applied in case of renamed types autoupgrade bit kicks in first and remangles the intrinsic. It happens because autoupgrade path also matches the name with the expected name for the given signature. It makes this case slightly convoluted but I don't see any other simple way to check if autoupgrade is needed. 

Since we know the old mangling scheme for the masked intrinsics we might try to match the name with the old mangling name.
  if (Name == Intrinsics::getName(masked_load, OldTys)
    // Upgrade
But it won't work in case when types were renamed.

Difficulties separating upgrade and remangling paths was one of the reasons I combined them both in autoupgrade in the initial revision.


http://reviews.llvm.org/D19373





More information about the llvm-commits mailing list