[PATCH] D124118: [Peephole-Opt] For one kind of test-after-add pattern, eliminates test if it's correct to do so.

[David Li via Phabricator via llvm-commits]

davidxl added a comment.

In D124118#3463462 <https://reviews.llvm.org/D124118#3463462>, @craig.topper wrote:

> In D124118#3463205 <https://reviews.llvm.org/D124118#3463205>, @davidxl wrote:
>
>> I suspect there is something else going on.
>>
>> The following case works fine without the patch.
>>
>> define dso_local noundef i64 @_Z3fooPlll(ptr nocapture noundef %0, i64 noundef %1, i64 noundef %2) local_unnamed_addr #0 {
>>
>>   %4 = load i64, ptr %0, align 8
>>   %5 = and i64 %4, 3
>>   %6 = icmp eq i64 %5, 0
>>   %7 = select i1 %6, i64 %1, i64 %2
>>   store i64 %7, ptr %0, align 8
>>   ret i64 %5
>>
>> }
>>
>> However a simple change below to replace the operand %2 in the select with %5 (output of and operation), then it stops working.
>>
>> define dso_local noundef i64 @_Z3fooPlll(ptr nocapture noundef %0, i64 noundef %1, i64 noundef %2) local_unnamed_addr #0 {
>>
>>   %4 = load i64, ptr %0, align 8
>>   %5 = and i64 %4, 3
>>   %6 = icmp eq i64 %5, 0
>>   %7 = select i1 %6, i64 %1, i64 %5
>>   store i64 %7, ptr %0, align 8
>>   ret i64 %5
>>
>> }
>>
>> Can you investigate the difference?  The behavior difference on ARM is also worth comparing with.
>
> There's a peephole in X86DAGToDAGISel::Select and PostProcessISelDAG that both fail if the AND has users other than the TEST.

thanks. That explains it.

For this case, the subreg_to_reg is indeed the reason blocking the peephole-opt from cleaning it up (as can be seen in the case if the AND operand is changed to 0x1ffffffff where subreg_to_reg is not generated)



================
Comment at: llvm/lib/Target/X86/X86InstrInfo.cpp:1043
+  if (X86::isAND(vregDefInstr->getOpcode()) ||
+      X86::isOR(vregDefInstr->getOpcode()) ||
+      X86::isXOR(vregDefInstr->getOpcode())) {
----------------
Is there need to handle OR and XOR here? It seems we won't see code patterns with OR followed by subreg_to_reg.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124118