[PATCH] D122471: [IR] Require intrinsic struct return type to be anonymous

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

@dexonsmith In the end, the part we care about is that that there is a unique mapping between intrinsic names and intrinsic function types. Even if we represented named structs as a type containing an anonymous struct and a name, that would still mean we could have two different function types for one intrinsic name. The only way to avoid that would be to actually include the struct type in intrinsic mangling, so we'd have a different intrinsic for (say) `llvm.sadd.with.overflow.i32.s_anon` returning an anonymous struct and `llvm.sadd.with.overflow.i32.s_0` returning a `%0` named struct. That's possible, but I don't think its worthwhile.

Conversely, if we move towards only having anonymous structs and a name assigned in a separate map, then that name is no longer part of the Type, and we only have the one intrinsic signature. The name is just a question of pretty rendering. Though TBH I am not sure how much sense that approach makes -- I think named types are primarily of interest if you can use different names for the "same" type, while this would limit you to one.

Generally, I'd say that struct types in LLVM are on the way out. Opaque pointers make the first major step towards that, and if we could get rid of all the other unnecessary type uses in LLVM (GEP, alloca, byval, etc) the only remaining use for struct types would be functions with multiple return values. But well, we're still rather far away from this bright future :)

> Or, maybe, names on struct types returned by intrinsics just aren't very valuable? (since they're rare / small / easy enough to reason about?)

Yeah, I think when it comes to IR intrinsics, struct returns are generally only used to return two values. I don't think people use named return types for this in practice anymore, because it doesn't really work for non-trivial situations (i.e. whenever LLVM creates new intrinsic calls, as it does with with.overflow intrinsics).


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

https://reviews.llvm.org/D122471