[PATCH] D118744: [clang] Fix some clang->llvm type cache invalidation issues

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks added a comment.

In D118744#3301977 <https://reviews.llvm.org/D118744#3301977>, @rnk wrote:

> This seems unfortunately complex, but I think we can live with it for a year or two.
>
> Is it possible to use the compile time tracker to benchmark if this clang->LLVM type cache actually saves compile time? This change disables the cache pretty often, and I'm wondering if "pretty often" is close enough to "all the time" to let us remove the cache altogether.

Yeah I measured disabling the cache and saw some slight regressions.

> ---
>
>> Since the LLVM type for z::p can't reference z, Clang simply treats z::p as a pointer to an empty struct {}*.
>
> I don't think this is strictly true. LLVM IR struct types can effectively be forward declared when building IR, using the two-phase `StructType::create/setBody` APIs. Consider the usual linked list example:
>
>   struct z { z *next; };
>   z head;
>
> -->
>
>   %struct.z = type { %struct.z* }
>   @"?head@@3Uz@@A" = dso_local local_unnamed_addr global %struct.z zeroinitializer, align 4, !dbg !0

Ah you're right, I'll update the description

> The IR for function prototypes can work similarly, we could produce this IR struct type:
>
>   %struct.z = type { (%struct.z (%struct.z))* }
>
> Maybe that's hard to do today in clang, and ultimately it's probably not worth investing in generating pretty (function) pointer types given the impending transition to opaque pointers.

There's a lot of code around choosing proper types (it's not as simple as mapping a C++ `struct z` to LLVM `%struct.z`), as you said it would take some time and it's probably not worth it given opaque pointers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118744