[PATCH] D148986: [InstSimplify] with logical ops: (X | Y) ? 0 : X --> 0

Nikita Popov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Apr 22 03:46:33 PDT 2023


nikic added a comment.

In D148986#4289607 <https://reviews.llvm.org/D148986#4289607>, @floatshadow wrote:

> In D148986#4289600 <https://reviews.llvm.org/D148986#4289600>, @nikic wrote:
>
>> As this is the second special case for this pattern, we should generalize it.
>>
>> 1. Extract https://github.com/llvm/llvm-project/blob/9ea3fcfa380c6097fddd0d9a9b2c13f0f20bc41a/llvm/lib/Analysis/InstructionSimplify.cpp#L4569-L4582 in one direction into a helper, and call it in both directions there.
>> 2. Add a special case for or equals zero and and equals minus one where we try the replacement with the operands of the or/and.
>> 3. Remove https://github.com/llvm/llvm-project/blob/9ea3fcfa380c6097fddd0d9a9b2c13f0f20bc41a/llvm/lib/Analysis/InstructionSimplify.cpp#L4585-L4598, which is subsumed.
>
> My concern mentioned in this issue is the case the condition of ternary operator contains multiple ORs/ANDs. There seems a need to do a recursive check (?)

Unless there is reason to believe that such patterns occur in practice, there is no need for premature over-generalization. There are lots of places where we //could// be doing recursive checks, but in most places that just unnecessarily complicates things without providing tangible benefit.


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

https://reviews.llvm.org/D148986



More information about the llvm-commits mailing list