[PATCH] D29925: Implement intrinsic mangling for literal struct types.Fixes PR 31921

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc added inline comments.


================
Comment at: lib/IR/AutoUpgrade.cpp:492-493
   }
+  // Remangle our intrinsic since we upgrade the mangling
+  auto Result = llvm::Intrinsic::remangleIntrinsicFunction(F);
+  if (Result != None) {
----------------
dberlin wrote:
> chandlerc wrote:
> > Scan the types to see if any are pointers to structs before doing this?
> Unfortunately, that won't work.
> In fact, that was my original workaround in predicateinfo that broke.
> There are two problems:
> 
> 1. ssa_copy is anytype, so it can take a literal struct directly, which will still hit the assert.
> 
> But hey, we could special case that one.
> 
> There's a much worse problem though:
> 2. The mangling is recursive over array types, struct type, and function arguments/return types, so it's not enough to identify pointers to structs.
> They can be embedded *anywhere*.
> 
> So, for example, a pointer to a function type that takes a literal struct as an argument would still hit the old assert (and still be changed in mangling)
> Or a pointer to function that has a return type of an array of pointers to functions that take literal structs ...
> ;(
> 
> 
> Essentially, the only way to avoid this check is either:
> 
> A. We try to name match all the broken intrinsics and add any we missed over time.
> B. We give up and just use it like it is.
> 
> I am happy to do A if we are concerned about the compile time cost
Wow. Just wow.

I wouldn't bother with name matching. The intrinsics i'm most concerned with will match (mem*).

Maybe we could check for iN and iN* parameters and skip it. Maybe not worth it.

Actually, yeah, why not just try auto upgrading a large bitcode file and see how much (if any) time is spent here? If none, we can revisit this if ever someone finds bitcode that has enough intrinsics to matter. I'm crossing my fingers and hoping that this just doesn't matter. =]


https://reviews.llvm.org/D29925