[PATCH] D97204: [RFC] Clang 64-bit source locations

lebedev.ri resigned from this revision. lebedev.ri added a comment. Herald added subscribers: steakhal, StephenFan. Herald added a reviewer: NoQ. Herald added a reviewer: njames93. This review may be stuck/dead, consider abandoning if no longer relevant. Removing myself as reviewer in attempt to

[PATCH] D97204: [RFC] Clang 64-bit source locations

lenary added a comment. Herald added a project: All. Some of this patch set has been broken by https://reviews.llvm.org/D125403 and https://reviews.llvm.org/D125952 - which are size optimisations for PCH encodings of source locations. The biggest issue is new line 90 of

[PATCH] D97204: [RFC] Clang 64-bit source locations

simon_tatham added a comment. This patch doesn't seem to have attracted much review attention in the last couple of weeks. On the theory that perhaps it's just too big and monolithic to review in one go, I've started to break it up into smaller pieces. So far I've only covered the preparation

[PATCH] D97204: [RFC] Clang 64-bit source locations

simon_tatham added a comment. Hmmm. Two people have pointed out to me that my strategy of having a 32-bit `SourceLocations::LowBits` and an 0- or 32-bit `SourceLocations::OptionalHighBits` doesn't actually work, because an empty struct still takes at least 1 byte. So this version of the patch

[PATCH] D97204: [RFC] Clang 64-bit source locations

simon_tatham added a comment. Thanks to @miyuki for repeating the previous benchmark with this version of the patch (and on the same machine as before, which was better than I could have done). The revised results now have the memory usage increase (compared to 32-bit SourceLocation) in the

[PATCH] D97204: [RFC] Clang 64-bit source locations

miyuki added a comment. In D97204#2586111 , @rsmith wrote: > Thanks for doing this! > > The 8-9% memory hit is better than I'd feared, but still seems uncomfortably > large. I've left comments on a couple of places where I think we could > substantially

[PATCH] D97204: [RFC] Clang 64-bit source locations

akyrtzi added a comment. In D97204#2586111 , @rsmith wrote: > Can we avoid a libclang ABI break if we don't allow the use of 64-bit source > locations for builds with 32-bit pointers? To @rsmith's point, the simplest option may be to avoid building

[PATCH] D97204: [RFC] Clang 64-bit source locations

rsmith added a comment. Thanks for doing this! The 8-9% memory hit is better than I'd feared, but still seems uncomfortably large. I've left comments on a couple of places where I think we could substantially reduce this. The performance regression seems large enough that people will notice,

[PATCH] D97204: [RFC] Clang 64-bit source locations

miyuki updated this revision to Diff 325762. miyuki edited the summary of this revision. miyuki added a comment. Fixed python bindings and formatting. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97204/new/ https://reviews.llvm.org/D97204 Files:

[PATCH] D97204: [RFC] Clang 64-bit source locations

miyuki added a comment. In D97204#2580490 , @akyrtzi wrote: > Hi @miyuki, > >> A major thing worth noting is that 64-bit source locations will >> require an ABI breakage in libclang. This patch changes the bit width >> in libclang unconditionally, rather

[PATCH] D97204: [RFC] Clang 64-bit source locations

akyrtzi added a comment. Hi @miyuki, > A major thing worth noting is that 64-bit source locations will > require an ABI breakage in libclang. This patch changes the bit width > in libclang unconditionally, rather than making it configurable. Is it possible to make the libclang change

[PATCH] D97204: [RFC] Clang 64-bit source locations

miyuki created this revision. miyuki added a reviewer: rsmith. Herald added subscribers: dexonsmith, lebedev.ri, martong, arphaman, kbarton, hiraditya, mgorny, nemanjai. Herald added a reviewer: lebedev.ri. miyuki requested review of this revision. Herald added projects: clang, LLVM. Herald added