[llvm] r306106 - [SystemZ] Fix trap issue and enable expensive checks.

Jonas Paulsson via llvm-commits llvm-commits at lists.llvm.org
Sat Jun 24 01:49:41 PDT 2017


Sorry for failing to check this properly. I did not expect that a target 
would actually run two verifier passes after each other after LSR with 
-verify-machineinstrs:

      Loop Pass Manager
         Induction Variable Users
         Loop Strength Reduction
       Verify generated machine code
       Verify generated machine code
       MIR Printing Pass

/Jonas


On 2017-06-23 20:41, Krzysztof Parzyszek wrote:
> The default architecture for the Generic tests may change from bot to 
> bot. The Hexagon bot only has the Hexagon target, and that's the 
> default architecture for it. When I run check-llvm on my computer, it 
> passes too, but if you run this test by hand and add -march=hexagon, 
> it will fail.
>
> This testcase depends on what TargetPassConfig does and it's not 
> guaranteed to do the exact same thing for each target.
>
> Also, if you look at the build before, it's green:
> http://lab.llvm.org:8011/builders/llvm-hexagon-elf/builds/7303
>
> Anyways, I know how to fix this. I'll commit a patch soon.
>
> -Krzysztof
>
>
> On 6/23/2017 12:57 PM, Jonas Paulsson wrote:
>> Hi Krzysztof,
>>
>> When I looked at the buildbots page, it looked like it was broken 
>> before my commit.
>>
>> If it was my commit that caused the problem on your side, that's 
>> strange, because I ran it with and without EXPENSIVE_CHECKS on 
>> SystemZ for all targets, which passed. I then also ran those tests on 
>> my laptop for X86, which worked fine.
>>
>> Since this is a generic test, there should not be any difference for 
>> any target. What I did was simply to add the machine-verifier to that 
>> test so that it will work properly regardless of EXPENSIVE_CHECKS. If 
>> building with EXPENSIVE_CHECKS, the machine verifier will be inserted 
>> in the pass list, and tests that check for the pass list output, or 
>> pass arguments list, did not work before...
>>
>> Please let me know if I can help in any way. CC:ing Simon who might 
>> be able to help,
>>
>> Jonas
>>
>>
>>
>> On 2017-06-23 19:49, Krzysztof Parzyszek wrote:
>>> Hi Jonas,
>>> This broke Hexagon bots:
>>>
>>> http://lab.llvm.org:8011/builders/llvm-hexagon-elf/builds/7304
>>>
>>> -Krzysztof
>>>
>>>
>>> On 6/23/2017 9:30 AM, Jonas Paulsson via llvm-commits wrote:
>>>> Author: jonpa
>>>> Date: Fri Jun 23 09:30:46 2017
>>>> New Revision: 306106
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=306106&view=rev
>>>> Log:
>>>> [SystemZ]  Fix trap issue and enable expensive checks.
>>>>
>>>> The isBarrier/isTerminator flags have been removed from the SystemZ 
>>>> trap
>>>> instructions, so that tests do not fail with EXPENSIVE_CHECKS. This 
>>>> was just
>>>> an issue at -O0 and did not affect code output on benchmarks.
>>>>
>>>> (Like Eli pointed out: "targets are split over whether they 
>>>> consider their
>>>> "trap" a terminator; x86, AArch64, and NVPTX don't, but ARM, MIPS, 
>>>> PPC, and
>>>> SystemZ do. We should probably try to be consistent here.". This is 
>>>> still the
>>>> case, although SystemZ has switched sides).
>>>>
>>>> SystemZ now returns true in isMachineVerifierClean() :-)
>>>>
>>>> These Generic tests have been modified so that they can be run with 
>>>> or without
>>>> EXPENSIVE_CHECKS: CodeGen/Generic/llc-start-stop.ll and
>>>> CodeGen/Generic/print-machineinstrs.ll
>>>>
>>>> Review: Ulrich Weigand, Simon Pilgrim, Eli Friedman
>>>> https://bugs.llvm.org/show_bug.cgi?id=33047
>>>> https://reviews.llvm.org/D34143
>>>>
>>>> Modified:
>>>>      llvm/trunk/lib/Target/SystemZ/SystemZInstrInfo.td
>>>>      llvm/trunk/lib/Target/SystemZ/SystemZTargetMachine.h
>>>>      llvm/trunk/test/CodeGen/Generic/llc-start-stop.ll
>>>>      llvm/trunk/test/CodeGen/Generic/print-machineinstrs.ll
>>>>      llvm/trunk/test/CodeGen/SystemZ/trap-02.ll
>>>>
>>>> Modified: llvm/trunk/lib/Target/SystemZ/SystemZInstrInfo.td
>>>> URL: 
>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/SystemZ/SystemZInstrInfo.td?rev=306106&r1=306105&r2=306106&view=diff 
>>>>
>>>> ============================================================================== 
>>>>
>>>> --- llvm/trunk/lib/Target/SystemZ/SystemZInstrInfo.td (original)
>>>> +++ llvm/trunk/lib/Target/SystemZ/SystemZInstrInfo.td Fri Jun 23 
>>>> 09:30:46 2017
>>>> @@ -189,18 +189,15 @@ let isBranch = 1, isTerminator = 1 in {
>>>> //===----------------------------------------------------------------------===// 
>>>>
>>>>     // Unconditional trap.
>>>> -// FIXME: This trap instruction should be marked as isTerminator, 
>>>> but there is
>>>> -// currently a general bug that allows non-terminators to be 
>>>> placed between
>>>> -// terminators. Temporarily leave this unmarked until the bug is 
>>>> fixed.
>>>> -let isBarrier = 1, hasCtrlDep = 1 in
>>>> +let hasCtrlDep = 1 in
>>>>     def Trap : Alias<4, (outs), (ins), [(trap)]>;
>>>>     // Conditional trap.
>>>> -let isTerminator = 1, hasCtrlDep = 1, Uses = [CC] in
>>>> +let hasCtrlDep = 1, Uses = [CC] in
>>>>     def CondTrap : Alias<4, (outs), (ins cond4:$valid, cond4:$R1), 
>>>> []>;
>>>>     // Fused compare-and-trap instructions.
>>>> -let isTerminator = 1, hasCtrlDep = 1 in {
>>>> +let hasCtrlDep = 1 in {
>>>>     // These patterns work the same way as for compare-and-branch.
>>>>     defm CRT   : CmpBranchRRFcPair<"crt",   0xB972, GR32>;
>>>>     defm CGRT  : CmpBranchRRFcPair<"cgrt",  0xB960, GR64>;
>>>>
>>>> Modified: llvm/trunk/lib/Target/SystemZ/SystemZTargetMachine.h
>>>> URL: 
>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/SystemZ/SystemZTargetMachine.h?rev=306106&r1=306105&r2=306106&view=diff 
>>>>
>>>> ============================================================================== 
>>>>
>>>> --- llvm/trunk/lib/Target/SystemZ/SystemZTargetMachine.h (original)
>>>> +++ llvm/trunk/lib/Target/SystemZ/SystemZTargetMachine.h Fri Jun 23 
>>>> 09:30:46 2017
>>>> @@ -51,8 +51,6 @@ public:
>>>>     }
>>>>       bool targetSchedulesPostRAScheduling() const override { 
>>>> return true; };
>>>> -
>>>> -  bool isMachineVerifierClean() const override { return false; }
>>>>   };
>>>>     } // end namespace llvm
>>>>
>>>> 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=306106&r1=306105&r2=306106&view=diff 
>>>>
>>>> ============================================================================== 
>>>>
>>>> --- llvm/trunk/test/CodeGen/Generic/llc-start-stop.ll (original)
>>>> +++ llvm/trunk/test/CodeGen/Generic/llc-start-stop.ll Fri Jun 23 
>>>> 09:30:46 2017
>>>> @@ -1,7 +1,10 @@
>>>> -; RUN: llc < %s -debug-pass=Structure -stop-after=loop-reduce -o 
>>>> /dev/null 2>&1 | FileCheck %s -check-prefix=STOP-AFTER
>>>> +; Note: -verify-machineinstrs is used in order to make this test 
>>>> compatible with EXPENSIVE_CHECKS.
>>>> +; RUN: llc < %s -debug-pass=Structure -stop-after=loop-reduce 
>>>> -verify-machineinstrs -o /dev/null 2>&1 \
>>>> +; RUN:   | FileCheck %s -check-prefix=STOP-AFTER
>>>>   ; STOP-AFTER: -loop-reduce
>>>>   ; STOP-AFTER: Dominator Tree Construction
>>>>   ; STOP-AFTER: Loop Strength Reduction
>>>> +; STOP-AFTER-NEXT: Verify generated machine code
>>>>   ; STOP-AFTER-NEXT: MIR Printing Pass
>>>>     ; RUN: llc < %s -debug-pass=Structure -stop-before=loop-reduce 
>>>> -o /dev/null 2>&1 | FileCheck %s -check-prefix=STOP-BEFORE
>>>>
>>>> Modified: llvm/trunk/test/CodeGen/Generic/print-machineinstrs.ll
>>>> URL: 
>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/Generic/print-machineinstrs.ll?rev=306106&r1=306105&r2=306106&view=diff 
>>>>
>>>> ============================================================================== 
>>>>
>>>> --- llvm/trunk/test/CodeGen/Generic/print-machineinstrs.ll (original)
>>>> +++ llvm/trunk/test/CodeGen/Generic/print-machineinstrs.ll Fri Jun 
>>>> 23 09:30:46 2017
>>>> @@ -1,12 +1,25 @@
>>>> -; RUN: llc < %s -O3 -debug-pass=Structure 
>>>> -print-machineinstrs=branch-folder -o /dev/null 2>&1 | FileCheck %s
>>>> -; RUN: llc < %s -O3 -debug-pass=Structure -print-machineinstrs -o 
>>>> /dev/null 2>&1 | FileCheck %s
>>>> -; RUN: llc < %s -O3 -debug-pass=Structure -print-machineinstrs= -o 
>>>> /dev/null 2>&1 | FileCheck %s
>>>> +; RUN: llc < %s -O3 -debug-pass=Structure 
>>>> -print-machineinstrs=branch-folder -verify-machineinstrs -o 
>>>> /dev/null 2>&1 \
>>>> +; RUN:   | FileCheck %s -check-prefix=PRINT-BRANCH-FOLD
>>>> +; RUN: llc < %s -O3 -debug-pass=Structure -print-machineinstrs 
>>>> -verify-machineinstrs -o /dev/null 2>&1 \
>>>> +; RUN:   | FileCheck %s -check-prefix=PRINT
>>>> +; RUN: llc < %s -O3 -debug-pass=Structure -print-machineinstrs= 
>>>> -verify-machineinstrs -o /dev/null 2>&1 \
>>>> +; RUN:   | FileCheck %s -check-prefix=PRINT
>>>> +
>>>> +; Note: -verify-machineinstrs is used in order to make this test 
>>>> compatible with EXPENSIVE_CHECKS.
>>>>     define i64 @foo(i64 %a, i64 %b) nounwind {
>>>> -; CHECK: -branch-folder  -machineinstr-printer
>>>> -; CHECK: Control Flow Optimizer
>>>> -; CHECK-NEXT: MachineFunction Printer
>>>> -; CHECK: Machine code for function foo:
>>>> +; PRINT-BRANCH-FOLD: -branch-folder -machineverifier 
>>>> -machineinstr-printer
>>>> +; PRINT-BRANCH-FOLD: Control Flow Optimizer
>>>> +; PRINT-BRANCH-FOLD-NEXT: Verify generated machine code
>>>> +; PRINT-BRANCH-FOLD-NEXT: MachineFunction Printer
>>>> +; PRINT-BRANCH-FOLD: Machine code for function foo:
>>>> +
>>>> +; PRINT: -branch-folder -machineinstr-printer
>>>> +; PRINT: Control Flow Optimizer
>>>> +; PRINT-NEXT: MachineFunction Printer
>>>> +; PRINT-NEXT: Verify generated machine code
>>>> +; PRINT: Machine code for function foo:
>>>> +
>>>>     %c = add i64 %a, %b
>>>>     %d = trunc i64 %c to i32
>>>>     %e = zext i32 %d to i64
>>>>
>>>> Modified: llvm/trunk/test/CodeGen/SystemZ/trap-02.ll
>>>> URL: 
>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/SystemZ/trap-02.ll?rev=306106&r1=306105&r2=306106&view=diff 
>>>>
>>>> ============================================================================== 
>>>>
>>>> --- llvm/trunk/test/CodeGen/SystemZ/trap-02.ll (original)
>>>> +++ llvm/trunk/test/CodeGen/SystemZ/trap-02.ll Fri Jun 23 09:30:46 
>>>> 2017
>>>> @@ -47,9 +47,9 @@ if.end:
>>>>   define i32 @f3(i32 zeroext %a, i32 *%base, i64 %offset) {
>>>>   ; CHECK-LABEL: f3:
>>>>   ; CHECK: cl %r2, 0(%r{{[0-5]}},%r3)
>>>> +; CHECK: lhi %r2, 0
>>>>   ; CHECK-LABEL: .Ltmp0
>>>>   ; CHECK: jh .Ltmp0+2
>>>> -; CHECK: lhi %r2, 0
>>>>   ; CHECK: br %r14
>>>>   entry:
>>>>     %ptr = getelementptr i32, i32 *%base, i64 %offset
>>>> @@ -70,9 +70,9 @@ if.end:
>>>>   define i64 @f4(i64 %a, i64 *%base, i64 %offset) {
>>>>   ; CHECK-LABEL: f4:
>>>>   ; CHECK: clg %r2, 0(%r{{[0-5]}},%r3)
>>>> +; CHECK: lghi %r2, 0
>>>>   ; CHECK-LABEL: .Ltmp1
>>>>   ; CHECK: jh .Ltmp1+2
>>>> -; CHECK: lghi %r2, 0
>>>>   ; CHECK: br %r14
>>>>   entry:
>>>>     %ptr = getelementptr i64, i64 *%base, i64 %offset
>>>>
>>>>
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at lists.llvm.org
>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>>
>>>
>>
>



More information about the llvm-commits mailing list