[llvm] 6102310 - [InstSimplify][EarlyCSE] Try to CSE PHI nodes in the same basic block

Philip Reames via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 1 09:18:40 PDT 2020


Thanks for chasing this down!

Philip

On 8/29/20 12:10 PM, Roman Lebedev wrote:
> Thank you Nikita, Philip and Owen!
>
> With iterative process it was deduced that the main culprit is actually
> the Instruction::isIdenticalToWhenDefined() change,
> because e.g. EliminateDuplicatePHINodes() expects it do be dumb,
> but (unlike EarlyCSE) did not verify it's invariants.
>
> I've added that missing invariant checking
> in 961483a5ea7c0e628c025187287d1658457ffcb3,
> and verified that it would have caught
> the Instruction::isIdenticalToWhenDefined() change.
>
> With InstCombine-based PHI CSE still in place,
> but Instruction::isIdenticalToWhenDefined() change reverted,
> the build is finally green again:
> http://lab.llvm.org:8011/builders/clang-with-thin-lto-ubuntu/builds/24100
>
> Since InstCombine-based PHI CSE, indeed, catches all the motivational cases,
> i'm *NOT* going to re-attempt InstSimpify-based approach.
>
> Roman.
>
> On Sat, Aug 29, 2020 at 2:35 AM Owen Anderson <resistor at mac.com> wrote:
>>
>>
>> On Aug 28, 2020, at 2:21 PM, Nikita Popov via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>>
>> As for actual breakage: I have a strong suspicion that this change has introduced compiler non-determinism. Looking at https://llvm-compile-time-tracker.com/index.php?config=ReleaseThinLTO&stat=size-text text size fluctuations have started occurring directly after this commit.
>>
>> Regards,
>> Nikita
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>>
>>
>> I’ve just finished bisecting a stage3 mismatch on x86 linux and tracked it down to this diff as well.  I’m going to move ahead with a revert.
>>
>> —Owen


More information about the llvm-commits mailing list