[llvm] r324319 - Revert "[MergeICmps] Enable the MergeICmps Pass by default."

Clement Courbet via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 6 01:29:00 PST 2018


On Tue, Feb 6, 2018 at 10:25 AM, Clement Courbet <courbet at google.com> wrote:

> Hi Chandler,
>
> Sorry about that, I'll make sure I'll ping the original commit and use the
> subversion revision next time.
>
> BTW: Is it OK to resubmit with more asserts and revert immediately ? I
> think I know what's happening, but I don't have a PPC to test. How do
> people usually debug that kind of things ?
>

Actually in this case the sanitizer builds confirmed what I though so I
don't need to, but I'd still like to know for next time :)


>
> On Tue, Feb 6, 2018 at 9:48 AM, Chandler Carruth <chandlerc at gmail.com>
> wrote:
>
>> Ok, last email I promise....
>>
>> Just realized I was super negative all over this thread, and that isn't
>> really warranted. Thanks for actually watching the bots and reverting the
>> patch when something came up! Totally saved me a bunch of time tracking
>> down the crashes I saw with Clang building itself with PGO enabled that
>> would have taken probably a lot of time.  Great that we had already gotten
>> back to green here.
>>
>> On Tue, Feb 6, 2018 at 12:47 AM Chandler Carruth <chandlerc at gmail.com>
>> wrote:
>>
>>> Also, in the future, *please* mention the svn revision you are
>>> reverting, not just the git commitish. Also useful to follow up on the
>>> original commit saying it has been reverted. Lots of folks are bisecting
>>> failures, and they may find the original commit and have no idea that it
>>> has been reverted.
>>>
>>> On Tue, Feb 6, 2018 at 12:45 AM Chandler Carruth <chandlerc at gmail.com>
>>> wrote:
>>>
>>>> It also appears to break numerous other self-host builds when using PGO
>>>> of any kind. Just FYI.
>>>>
>>>> On Tue, Feb 6, 2018 at 12:41 AM Clement Courbet via llvm-commits <
>>>> llvm-commits at lists.llvm.org> wrote:
>>>>
>>>>> Author: courbet
>>>>> Date: Tue Feb  6 00:40:18 2018
>>>>> New Revision: 324319
>>>>>
>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=324319&view=rev
>>>>> Log:
>>>>> Revert "[MergeICmps] Enable the MergeICmps Pass by default."
>>>>>
>>>>> Breaks clang-ppc64be-linux-multistage buildbot.
>>>>>
>>>>> This reverts commit 515bab711f308c2e8299c49dd8c84ea6a2e0b60e.
>>>>>
>>>>> Modified:
>>>>>     llvm/trunk/lib/CodeGen/TargetPassConfig.cpp
>>>>>     llvm/trunk/test/CodeGen/Generic/llc-start-stop.ll
>>>>>     llvm/trunk/test/CodeGen/PowerPC/memcmp-mergeexpand.ll
>>>>>     llvm/trunk/test/CodeGen/X86/memcmp-mergeexpand.ll
>>>>>
>>>>> Modified: llvm/trunk/lib/CodeGen/TargetPassConfig.cpp
>>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/T
>>>>> argetPassConfig.cpp?rev=324319&r1=324318&r2=324319&view=diff
>>>>> ============================================================
>>>>> ==================
>>>>> --- llvm/trunk/lib/CodeGen/TargetPassConfig.cpp (original)
>>>>> +++ llvm/trunk/lib/CodeGen/TargetPassConfig.cpp Tue Feb  6 00:40:18
>>>>> 2018
>>>>> @@ -94,9 +94,10 @@ static cl::opt<bool> EnableImplicitNullC
>>>>>      "enable-implicit-null-checks",
>>>>>      cl::desc("Fold null checks into faulting memory operations"),
>>>>>      cl::init(false), cl::Hidden);
>>>>> -static cl::opt<bool> DisableMergeICmps("disable-mergeicmps",
>>>>> -    cl::desc("Disable MergeICmps Pass"),
>>>>> -    cl::init(false), cl::Hidden);
>>>>> +static cl::opt<bool>
>>>>> +    EnableMergeICmps("enable-mergeicmps",
>>>>> +                     cl::desc("Merge ICmp chains into a single
>>>>> memcmp"),
>>>>> +                     cl::init(false), cl::Hidden);
>>>>>  static cl::opt<bool> PrintLSR("print-lsr-output", cl::Hidden,
>>>>>      cl::desc("Print LLVM IR produced by the loop-reduce pass"));
>>>>>  static cl::opt<bool> PrintISelInput("print-isel-input", cl::Hidden,
>>>>> @@ -595,7 +596,7 @@ void TargetPassConfig::addIRPasses() {
>>>>>      // loads and compares. ExpandMemCmpPass then tries to expand
>>>>> those calls
>>>>>      // into optimally-sized loads and compares. The transforms are
>>>>> enabled by a
>>>>>      // target lowering hook.
>>>>> -    if (!DisableMergeICmps)
>>>>> +    if (EnableMergeICmps)
>>>>>        addPass(createMergeICmpsPass());
>>>>>      addPass(createExpandMemCmpPass());
>>>>>    }
>>>>>
>>>>> Modified: llvm/trunk/test/CodeGen/Generic/llc-start-stop.ll
>>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/
>>>>> Generic/llc-start-stop.ll?rev=324319&r1=324318&r2=324319&view=diff
>>>>> ============================================================
>>>>> ==================
>>>>> --- llvm/trunk/test/CodeGen/Generic/llc-start-stop.ll (original)
>>>>> +++ llvm/trunk/test/CodeGen/Generic/llc-start-stop.ll Tue Feb  6
>>>>> 00:40:18 2018
>>>>> @@ -13,15 +13,15 @@
>>>>>  ; STOP-BEFORE-NOT: Loop Strength Reduction
>>>>>
>>>>>  ; RUN: llc < %s -debug-pass=Structure -start-after=loop-reduce -o
>>>>> /dev/null 2>&1 | FileCheck %s -check-prefix=START-AFTER
>>>>> -; START-AFTER: -machine-branch-prob -mergeicmps
>>>>> +; START-AFTER: -machine-branch-prob -expandmemcmp
>>>>>  ; START-AFTER: FunctionPass Manager
>>>>> -; START-AFTER-NEXT: Merge contiguous icmps into a memcmp
>>>>> +; START-AFTER-NEXT: Expand memcmp() to load/stores
>>>>>
>>>>>  ; RUN: llc < %s -debug-pass=Structure -start-before=loop-reduce -o
>>>>> /dev/null 2>&1 | FileCheck %s -check-prefix=START-BEFORE
>>>>>  ; START-BEFORE: -machine-branch-prob -domtree
>>>>>  ; START-BEFORE: FunctionPass Manager
>>>>>  ; START-BEFORE: Loop Strength Reduction
>>>>> -; START-BEFORE-NEXT: Merge contiguous icmps into a memcmp
>>>>> +; START-BEFORE-NEXT: Expand memcmp() to load/stores
>>>>>
>>>>>  ; RUN: not llc < %s -start-before=nonexistent -o /dev/null 2>&1 |
>>>>> FileCheck %s -check-prefix=NONEXISTENT-START-BEFORE
>>>>>  ; RUN: not llc < %s -stop-before=nonexistent -o /dev/null 2>&1 |
>>>>> FileCheck %s -check-prefix=NONEXISTENT-STOP-BEFORE
>>>>>
>>>>> Modified: llvm/trunk/test/CodeGen/PowerPC/memcmp-mergeexpand.ll
>>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/
>>>>> PowerPC/memcmp-mergeexpand.ll?rev=324319&r1=324318&r2=324319&view=diff
>>>>> ============================================================
>>>>> ==================
>>>>> --- llvm/trunk/test/CodeGen/PowerPC/memcmp-mergeexpand.ll (original)
>>>>> +++ llvm/trunk/test/CodeGen/PowerPC/memcmp-mergeexpand.ll Tue Feb  6
>>>>> 00:40:18 2018
>>>>> @@ -7,12 +7,22 @@
>>>>>
>>>>>  define zeroext i1 @opeq1(
>>>>>  ; PPC64LE-LABEL: opeq1:
>>>>> -; PPC64LE:       # %bb.0: # %opeq1.exit
>>>>> -; PPC64LE-NEXT:    ld 3, 0(3)
>>>>> -; PPC64LE-NEXT:    ld 4, 0(4)
>>>>> -; PPC64LE-NEXT:    xor 3, 3, 4
>>>>> -; PPC64LE-NEXT:    cntlzd 3, 3
>>>>> -; PPC64LE-NEXT:    rldicl 3, 3, 58, 63
>>>>> +; PPC64LE:       # %bb.0: # %entry
>>>>> +; PPC64LE-NEXT:    lwz 5, 0(3)
>>>>> +; PPC64LE-NEXT:    lwz 6, 0(4)
>>>>> +; PPC64LE-NEXT:    cmplw 5, 6
>>>>> +; PPC64LE-NEXT:    bne 0, .LBB0_2
>>>>> +; PPC64LE-NEXT:  # %bb.1: # %land.rhs.i
>>>>> +; PPC64LE-NEXT:    lwz 3, 4(3)
>>>>> +; PPC64LE-NEXT:    lwz 4, 4(4)
>>>>> +; PPC64LE-NEXT:    cmpw 3, 4
>>>>> +; PPC64LE-NEXT:    b .LBB0_3
>>>>> +; PPC64LE-NEXT:  .LBB0_2:
>>>>> +; PPC64LE-NEXT:    crxor 2, 2, 2
>>>>> +; PPC64LE-NEXT:  .LBB0_3: # %opeq1.exit
>>>>> +; PPC64LE-NEXT:    li 3, 0
>>>>> +; PPC64LE-NEXT:    li 4, 1
>>>>> +; PPC64LE-NEXT:    isel 3, 4, 3, 2
>>>>>  ; PPC64LE-NEXT:    blr
>>>>>    %"struct.std::pair"* nocapture readonly dereferenceable(8) %a,
>>>>>    %"struct.std::pair"* nocapture readonly dereferenceable(8) %b)
>>>>> local_unnamed_addr #0 {
>>>>>
>>>>> Modified: llvm/trunk/test/CodeGen/X86/memcmp-mergeexpand.ll
>>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/
>>>>> X86/memcmp-mergeexpand.ll?rev=324319&r1=324318&r2=324319&view=diff
>>>>> ============================================================
>>>>> ==================
>>>>> --- llvm/trunk/test/CodeGen/X86/memcmp-mergeexpand.ll (original)
>>>>> +++ llvm/trunk/test/CodeGen/X86/memcmp-mergeexpand.ll Tue Feb  6
>>>>> 00:40:18 2018
>>>>> @@ -8,22 +8,37 @@
>>>>>
>>>>>  define zeroext i1 @opeq1(
>>>>>  ; X86-LABEL: opeq1:
>>>>> -; X86:       # %bb.0: # %opeq1.exit
>>>>> +; X86:       # %bb.0: # %entry
>>>>>  ; X86-NEXT:    movl {{[0-9]+}}(%esp), %eax
>>>>>  ; X86-NEXT:    movl {{[0-9]+}}(%esp), %ecx
>>>>>  ; X86-NEXT:    movl (%ecx), %edx
>>>>> +; X86-NEXT:    cmpl (%eax), %edx
>>>>> +; X86-NEXT:    jne .LBB0_1
>>>>> +; X86-NEXT:  # %bb.2: # %land.rhs.i
>>>>>  ; X86-NEXT:    movl 4(%ecx), %ecx
>>>>> -; X86-NEXT:    xorl (%eax), %edx
>>>>> -; X86-NEXT:    xorl 4(%eax), %ecx
>>>>> -; X86-NEXT:    orl %edx, %ecx
>>>>> +; X86-NEXT:    cmpl 4(%eax), %ecx
>>>>>  ; X86-NEXT:    sete %al
>>>>> +; X86-NEXT:    # kill: def $al killed $al killed $eax
>>>>> +; X86-NEXT:    retl
>>>>> +; X86-NEXT:  .LBB0_1:
>>>>> +; X86-NEXT:    xorl %eax, %eax
>>>>> +; X86-NEXT:    # kill: def $al killed $al killed $eax
>>>>>  ; X86-NEXT:    retl
>>>>>  ;
>>>>>  ; X64-LABEL: opeq1:
>>>>> -; X64:       # %bb.0: # %opeq1.exit
>>>>> -; X64-NEXT:    movq (%rdi), %rax
>>>>> -; X64-NEXT:    cmpq (%rsi), %rax
>>>>> +; X64:       # %bb.0: # %entry
>>>>> +; X64-NEXT:    movl (%rdi), %eax
>>>>> +; X64-NEXT:    cmpl (%rsi), %eax
>>>>> +; X64-NEXT:    jne .LBB0_1
>>>>> +; X64-NEXT:  # %bb.2: # %land.rhs.i
>>>>> +; X64-NEXT:    movl 4(%rdi), %eax
>>>>> +; X64-NEXT:    cmpl 4(%rsi), %eax
>>>>>  ; X64-NEXT:    sete %al
>>>>> +; X64-NEXT:    # kill: def $al killed $al killed $eax
>>>>> +; X64-NEXT:    retq
>>>>> +; X64-NEXT:  .LBB0_1:
>>>>> +; X64-NEXT:    xorl %eax, %eax
>>>>> +; X64-NEXT:    # kill: def $al killed $al killed $eax
>>>>>  ; X64-NEXT:    retq
>>>>>    %"struct.std::pair"* nocapture readonly dereferenceable(8) %a,
>>>>>    %"struct.std::pair"* nocapture readonly dereferenceable(8) %b)
>>>>> local_unnamed_addr #0 {
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> llvm-commits mailing list
>>>>> llvm-commits at lists.llvm.org
>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>>>
>>>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180206/e92650b0/attachment.html>


More information about the llvm-commits mailing list