[PATCH] D142803: [LogicCombine 1/?] Implement a general way to simplify logical operations.

Fri Feb 24 07:48:59 PST 2023

spatel added a comment.

In D142803#4149087 <https://reviews.llvm.org/D142803#4149087>, @bcl5980 wrote:

> In D142803#4148486 <https://reviews.llvm.org/D142803#4148486>, @nikic wrote:
>
>> I'm concerned about the caching here. It looks like you reuse one LogicCombiner instance for a basic block. However, isn't it possible for some of the instructions that have been inserted into LogicalOpNodes to be deleted, in which case the map may contain dangling pointers. If the pointer is reused by a newly allocated instruction, the cached information will be incorrect.
>
> The main reason for caching is saving compile time. The new patch will remove all the instructions already inserted into the caches and I think functional it works now.
> @nikic @spatel if possible can we use the llvm-compile-time-track to test how much compile time increase if we enable the LogicCombiner for every single instruction?

If this -- https://github.com/llvm/llvm-project/commit/efcf6c2b1f8490a9258d40abb90b21da60a15919 -- is the experiment that you wanted to try, it seems to have no significant difference:
https://llvm-compile-time-tracker.com/compare.php?from=3592d05438acc1034905feff7ff555f4fd4c5774&to=efcf6c2b1f8490a9258d40abb90b21da60a15919&stat=instructions:u

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

https://reviews.llvm.org/D142803