[PowerPC] Turn on CR-Logical reducer pass

[nemanjai]

Summary

Quite a while ago, we implemented a pass that will reduce the number of CR-logical operations we emit. It does so by converting a CR-logical operation into a branch. We have kept this off by default because it seemed to cause a significant regression with one benchmark.
However, that regression turned out to be due to a completely unrelated reason - AADB introducing a self-copy that is a priority-setting nop and it was just exacerbated by this pass.

Now that we understand the reason for the only degradation, we can turn this pass on by default. A subsequent patch will detect and remove self-copies in the pre-emit peephole.

[hfinkel]

Now that we understand the reason for the only degradation, we can turn this pass on by default. A subsequent patch will detect and remove self-copies in the pre-emit peephole.

SGTM.

[RKSimon]

@nemanjai This is breaking on EXPENSIVE_CHECKS buildbots - please can you take a look? http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/20263/steps/test-check-all/logs/stdio

[nemanjai]

In D52431#1714255, @RKSimon wrote:

@nemanjai This is breaking on EXPENSIVE_CHECKS buildbots - please can you take a look? http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/20263/steps/test-check-all/logs/stdio

Sorry about that. I'll have a look. I don't think it is the cause of the AMDGPU failures though.

[nemanjai]

In D52431#1714330, @nemanjai wrote:

In D52431#1714255, @RKSimon wrote:

@nemanjai This is breaking on EXPENSIVE_CHECKS buildbots - please can you take a look? http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/20263/steps/test-check-all/logs/stdio

Sorry about that. I'll have a look. I don't think it is the cause of the AMDGPU failures though.

Turns out I cannot build with EXPENSIVE_CHECKS turned on with my setup on PPC Linux. I'll revert the change until I can get this sorted out.

[RKSimon]

In D52431#1714330, @nemanjai wrote:

In D52431#1714255, @RKSimon wrote:

@nemanjai This is breaking on EXPENSIVE_CHECKS buildbots - please can you take a look? http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/20263/steps/test-check-all/logs/stdio

Sorry about that. I'll have a look. I don't think it is the cause of the AMDGPU failures though.

No, they were already a known problem: https://bugs.llvm.org/show_bug.cgi?id=43699

[nemanjai]

@RKSimon What are the chances I could get access to a machine where these issues can be reproduced? I have tried building with EXPENSIVE_CHECKS on PPC Linux, but that fails and I tried it on my MacBook but I am not seeing these failures with the patch applied.

OTOH, I see that on that bot, you build a debug build. I wonder if this failure only happens on debug builds with EXPENSIVE_CHECKS... I'll try that on my machine now.

[RKSimon]

@nemanjai Any luck on this? It seems to be something to do with PPCReduceCRLogicals::splitBlockOnBinaryCROp AllCRLogicalOps.push_back() calls, resulting in a reallocation and the original CRLogicalOpInfo &CRI reference being lost. Should/can the CRLogicalOpInfo& be passed by value instead?

[RKSimon]

This was reverted in rL375233

[nemanjai]

In D52431#1715629, @RKSimon wrote:

@nemanjai Any luck on this? It seems to be something to do with PPCReduceCRLogicals::splitBlockOnBinaryCROp AllCRLogicalOps.push_back() calls, resulting in a reallocation and the original CRLogicalOpInfo &CRI reference being lost. Should/can the CRLogicalOpInfo& be passed by value instead?

Ah, this makes perfect sense. Thanks Simon. I'll try to put together a reproducer and a fix.

[nemanjai]

A few updates to the CR Logical Reducer in addition to the pulled patch that turns it on:

• Do not use std::vector as there is no real reason for it
• Do not take references into a growable data structure :(

@RKSimon Simon, could you do me a big favour and try this out on the machine on which you saw the failures. I am not sure if this is the (only) issue but references into a growable data structure certainly should be removed. Thanks for spotting this issue - I don't know what I was thinking when I originally wrote this a long time ago.

[RKSimon]

In D52431#1715683, @nemanjai wrote:

A few updates to the CR Logical Reducer in addition to the pulled patch that turns it on:

• Do not use std::vector as there is no real reason for it
• Do not take references into a growable data structure :(

@RKSimon Simon, could you do me a big favour and try this out on the machine on which you saw the failures. I am not sure if this is the (only) issue but references into a growable data structure certainly should be removed. Thanks for spotting this issue - I don't know what I was thinking when I originally wrote this a long time ago.

I'll test it out but I'll be travelling to the devmtg so its not going to be until Monday at the earliest.

[RKSimon]

Thanks @nemanjai this no longer asserts/crashes on my windows build

[nemanjai]

In D52431#1716969, @RKSimon wrote:

Thanks @nemanjai this no longer asserts/crashes on my windows build

Thank you so much for all your help sorting this out Simon.

lib/Target/PowerPC/PPCTargetMachine.cpp

 static cl::opt<bool>
 EnableMachineCombinerPass("ppc-machine-combiner",
                           cl::desc("Enable the machine combiner pass"),
                           cl::init(true), cl::Hidden);
 
 static cl::opt<bool>
   ReduceCRLogical("ppc-reduce-cr-logicals",
                   cl::desc("Expand eligible cr-logical binary ops to branches"),
-                  cl::init(false), cl::Hidden);
+                  cl::init(true), cl::Hidden);
 extern "C" void LLVMInitializePowerPCTarget() {
   // Register the targets
   RegisterTargetMachine<PPCTargetMachine> A(getThePPC32Target());
   RegisterTargetMachine<PPCTargetMachine> B(getThePPC64Target());
   RegisterTargetMachine<PPCTargetMachine> C(getThePPC64LETarget());
 
   PassRegistry &PR = *PassRegistry::getPassRegistry();
   initializePPCBoolRetToIntPass(PR);

test/CodeGen/PowerPC/2008-10-28-f128-i32.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
-; RUN: llc -verify-machineinstrs < %s -mtriple=ppc32-- -o - | FileCheck %s
+; RUN: llc -verify-machineinstrs < %s -mtriple=powerpc-unknown-unknown -o - | FileCheck %s
 
 define i64 @__fixunstfdi(ppc_fp128 %a) nounwind readnone {
 ; CHECK-LABEL: __fixunstfdi:
 ; CHECK:       # %bb.0: # %entry
 ; CHECK-NEXT:    mflr 0
 ; CHECK-NEXT:    stw 0, 4(1)
 ; CHECK-NEXT:    stwu 1, -464(1)
 ; CHECK-NEXT:    mfcr 12

 ; CHECK-NEXT:    lwz 4, 388(1)
 ; CHECK-NEXT:    stw 4, 404(1)
 ; CHECK-NEXT:    lwz 3, 384(1)
 ; CHECK-NEXT:    stw 3, 400(1)
 ; CHECK-NEXT:    bc 4, 20, .LBB0_2
 ; CHECK-NEXT:  # %bb.1: # %bb5
 ; CHECK-NEXT:    li 3, 0
 ; CHECK-NEXT:    li 4, 0
-; CHECK-NEXT:    b .LBB0_16
+; CHECK-NEXT:    b .LBB0_17
 ; CHECK-NEXT:  .LBB0_2: # %bb1
 ; CHECK-NEXT:    lfd 0, 400(1)
 ; CHECK-NEXT:    lfd 1, 392(1)
 ; CHECK-NEXT:    li 29, 0
 ; CHECK-NEXT:    lis 3, 15856
 ; CHECK-NEXT:    stfd 0, 304(1)
 ; CHECK-NEXT:    stfd 1, 296(1)
 ; CHECK-NEXT:    stw 29, 340(1)

 ; CHECK-NEXT:    lwz 3, 196(1)
 ; CHECK-NEXT:    stw 3, 252(1)
 ; CHECK-NEXT:    lwz 3, 192(1)
 ; CHECK-NEXT:    stw 3, 248(1)
 ; CHECK-NEXT:    lfd 3, 256(1)
 ; CHECK-NEXT:    lfd 4, 248(1)
 ; CHECK-NEXT:    bl __gcc_qsub@PLT
 ; CHECK-NEXT:    stfd 2, 176(1)
-; CHECK-NEXT:    fcmpu 0, 2, 27
+; CHECK-NEXT:    fcmpu 1, 2, 27
 ; CHECK-NEXT:    stfd 1, 168(1)
-; CHECK-NEXT:    fcmpu 1, 1, 27
+; CHECK-NEXT:    fcmpu 0, 1, 27
 ; CHECK-NEXT:    lwz 3, 180(1)
-; CHECK-NEXT:    crandc 20, 6, 0
-; CHECK-NEXT:    cror 21, 5, 7
-; CHECK-NEXT:    cror 20, 21, 20
+; CHECK-NEXT:    crandc 20, 2, 4
 ; CHECK-NEXT:    stw 3, 268(1)
 ; CHECK-NEXT:    lwz 3, 176(1)
 ; CHECK-NEXT:    stw 3, 264(1)
 ; CHECK-NEXT:    lwz 3, 172(1)
 ; CHECK-NEXT:    stw 3, 276(1)
 ; CHECK-NEXT:    lwz 3, 168(1)
 ; CHECK-NEXT:    stw 3, 272(1)
 ; CHECK-NEXT:    lfd 30, 264(1)
 ; CHECK-NEXT:    lfd 31, 272(1)
-; CHECK-NEXT:    bc 12, 20, .LBB0_13
-; CHECK-NEXT:  # %bb.10: # %bb2
+; CHECK-NEXT:    bc 12, 20, .LBB0_14
+; CHECK-NEXT:  # %bb.10: # %bb1
+; CHECK-NEXT:    cror 20, 1, 3
+; CHECK-NEXT:    bc 12, 20, .LBB0_14
+; CHECK-NEXT:  # %bb.11: # %bb2
 ; CHECK-NEXT:    fneg 29, 30
 ; CHECK-NEXT:    fneg 28, 31
 ; CHECK-NEXT:    li 29, 0
 ; CHECK-NEXT:    lis 3, 16864
 ; CHECK-NEXT:    stfd 28, 48(1)
 ; CHECK-NEXT:    stfd 29, 40(1)
 ; CHECK-NEXT:    stw 29, 84(1)
 ; CHECK-NEXT:    stw 3, 80(1)

 ; CHECK-NEXT:    fctiwz 2, 2
 ; CHECK-NEXT:    stfd 2, 24(1)
 ; CHECK-NEXT:    fcmpu 1, 31, 1
 ; CHECK-NEXT:    lwz 3, 36(1)
 ; CHECK-NEXT:    lwz 4, 28(1)
 ; CHECK-NEXT:    crandc 20, 6, 1
 ; CHECK-NEXT:    cror 20, 4, 20
 ; CHECK-NEXT:    addis 3, 3, -32768
-; CHECK-NEXT:    bc 12, 20, .LBB0_12
-; CHECK-NEXT:  # %bb.11: # %bb2
+; CHECK-NEXT:    bc 12, 20, .LBB0_13
+; CHECK-NEXT:  # %bb.12: # %bb2
 ; CHECK-NEXT:    ori 3, 4, 0
-; CHECK-NEXT:    b .LBB0_12
-; CHECK-NEXT:  .LBB0_12: # %bb2
+; CHECK-NEXT:    b .LBB0_13
+; CHECK-NEXT:  .LBB0_13: # %bb2
 ; CHECK-NEXT:    subfic 4, 3, 0
 ; CHECK-NEXT:    subfe 3, 29, 30
-; CHECK-NEXT:    b .LBB0_16
-; CHECK-NEXT:  .LBB0_13: # %bb3
+; CHECK-NEXT:    b .LBB0_17
+; CHECK-NEXT:  .LBB0_14: # %bb3
 ; CHECK-NEXT:    li 3, 0
 ; CHECK-NEXT:    lis 4, 16864
 ; CHECK-NEXT:    stfd 31, 112(1)
 ; CHECK-NEXT:    stfd 30, 104(1)
 ; CHECK-NEXT:    stw 3, 148(1)
 ; CHECK-NEXT:    stw 4, 144(1)
 ; CHECK-NEXT:    stw 3, 140(1)
 ; CHECK-NEXT:    stw 3, 136(1)

 ; CHECK-NEXT:    fctiwz 2, 2
 ; CHECK-NEXT:    stfd 2, 88(1)
 ; CHECK-NEXT:    fcmpu 1, 31, 1
 ; CHECK-NEXT:    lwz 3, 100(1)
 ; CHECK-NEXT:    lwz 4, 92(1)
 ; CHECK-NEXT:    crandc 20, 6, 0
 ; CHECK-NEXT:    cror 20, 5, 20
 ; CHECK-NEXT:    addis 3, 3, -32768
-; CHECK-NEXT:    bc 12, 20, .LBB0_14
-; CHECK-NEXT:    b .LBB0_15
-; CHECK-NEXT:  .LBB0_14: # %bb3
-; CHECK-NEXT:    addi 4, 3, 0
+; CHECK-NEXT:    bc 12, 20, .LBB0_15
+; CHECK-NEXT:    b .LBB0_16
 ; CHECK-NEXT:  .LBB0_15: # %bb3
+; CHECK-NEXT:    addi 4, 3, 0
+; CHECK-NEXT:  .LBB0_16: # %bb3
 ; CHECK-NEXT:    mr 3, 30
-; CHECK-NEXT:  .LBB0_16: # %bb5
+; CHECK-NEXT:  .LBB0_17: # %bb5
 ; CHECK-NEXT:    lfd 31, 456(1) # 8-byte Folded Reload
 ; CHECK-NEXT:    lfd 30, 448(1) # 8-byte Folded Reload
 ; CHECK-NEXT:    lfd 29, 440(1) # 8-byte Folded Reload
 ; CHECK-NEXT:    lfd 28, 432(1) # 8-byte Folded Reload
 ; CHECK-NEXT:    lwz 12, 408(1)
 ; CHECK-NEXT:    lfd 27, 424(1) # 8-byte Folded Reload
 ; CHECK-NEXT:    lwz 30, 416(1) # 4-byte Folded Reload
 ; CHECK-NEXT:    mtcrf 32, 12 # cr2