[llvm] r297871 - ARM: avoid clobbering register in v6 jump-table expansion.

Matthias Braun via llvm-commits llvm-commits at lists.llvm.org
Tue May 30 11:56:04 PDT 2017


Why did you sneak the NoVRegs property back in to .mir files?

I had actually spend some time before to remove things that can be inferred (currently NoVRegs, NoPhis, isSSA) so we wouldn't need to have them in the .mir files.
Is the inference wrong in some case or do you want to test for them after printing a .mir file?

- Matthias

> On Mar 15, 2017, at 11:38 AM, Tim Northover via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
> Author: tnorthover
> Date: Wed Mar 15 13:38:13 2017
> New Revision: 297871
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=297871&view=rev
> Log:
> ARM: avoid clobbering register in v6 jump-table expansion.
> 
> If we got unlucky with register allocation and actual constpool placement, we
> could end up producing a tTBB_JT with an index that's already been clobbered.
> 
> Technically, we might be able to fix this situation up with a MOV, but I think
> the constant islands pass is complex enough without having to deal with more
> weird edge-cases.
> 
> Added:
>    llvm/trunk/test/CodeGen/ARM/v6-jumptable-clobber.mir
> Modified:
>    llvm/trunk/include/llvm/CodeGen/MIRYamlMapping.h
>    llvm/trunk/lib/CodeGen/MIRParser/MIRParser.cpp
>    llvm/trunk/lib/CodeGen/MIRPrinter.cpp
>    llvm/trunk/lib/Target/ARM/ARMConstantIslandPass.cpp
>    llvm/trunk/test/CodeGen/X86/GlobalISel/irtranslator-call.ll
> 
> Modified: llvm/trunk/include/llvm/CodeGen/MIRYamlMapping.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/MIRYamlMapping.h?rev=297871&r1=297870&r2=297871&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/CodeGen/MIRYamlMapping.h (original)
> +++ llvm/trunk/include/llvm/CodeGen/MIRYamlMapping.h Wed Mar 15 13:38:13 2017
> @@ -381,6 +381,7 @@ struct MachineFunction {
>   StringRef Name;
>   unsigned Alignment = 0;
>   bool ExposesReturnsTwice = false;
> +  bool NoVRegs;
>   // GISel MachineFunctionProperties.
>   bool Legalized = false;
>   bool RegBankSelected = false;
> @@ -405,6 +406,7 @@ template <> struct MappingTraits<Machine
>     YamlIO.mapRequired("name", MF.Name);
>     YamlIO.mapOptional("alignment", MF.Alignment);
>     YamlIO.mapOptional("exposesReturnsTwice", MF.ExposesReturnsTwice);
> +    YamlIO.mapOptional("noVRegs", MF.NoVRegs);
>     YamlIO.mapOptional("legalized", MF.Legalized);
>     YamlIO.mapOptional("regBankSelected", MF.RegBankSelected);
>     YamlIO.mapOptional("selected", MF.Selected);
> 
> Modified: llvm/trunk/lib/CodeGen/MIRParser/MIRParser.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MIRParser/MIRParser.cpp?rev=297871&r1=297870&r2=297871&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/MIRParser/MIRParser.cpp (original)
> +++ llvm/trunk/lib/CodeGen/MIRParser/MIRParser.cpp Wed Mar 15 13:38:13 2017
> @@ -332,6 +332,8 @@ bool MIRParserImpl::initializeMachineFun
>     MF.setAlignment(YamlMF.Alignment);
>   MF.setExposesReturnsTwice(YamlMF.ExposesReturnsTwice);
> 
> +  if (YamlMF.NoVRegs)
> +    MF.getProperties().set(MachineFunctionProperties::Property::NoVRegs);
>   if (YamlMF.Legalized)
>     MF.getProperties().set(MachineFunctionProperties::Property::Legalized);
>   if (YamlMF.RegBankSelected)
> 
> Modified: llvm/trunk/lib/CodeGen/MIRPrinter.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MIRPrinter.cpp?rev=297871&r1=297870&r2=297871&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/MIRPrinter.cpp (original)
> +++ llvm/trunk/lib/CodeGen/MIRPrinter.cpp Wed Mar 15 13:38:13 2017
> @@ -175,6 +175,8 @@ void MIRPrinter::print(const MachineFunc
>   YamlMF.Alignment = MF.getAlignment();
>   YamlMF.ExposesReturnsTwice = MF.exposesReturnsTwice();
> 
> +  YamlMF.NoVRegs = MF.getProperties().hasProperty(
> +      MachineFunctionProperties::Property::NoVRegs);
>   YamlMF.Legalized = MF.getProperties().hasProperty(
>       MachineFunctionProperties::Property::Legalized);
>   YamlMF.RegBankSelected = MF.getProperties().hasProperty(
> 
> Modified: llvm/trunk/lib/Target/ARM/ARMConstantIslandPass.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMConstantIslandPass.cpp?rev=297871&r1=297870&r2=297871&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/ARM/ARMConstantIslandPass.cpp (original)
> +++ llvm/trunk/lib/Target/ARM/ARMConstantIslandPass.cpp Wed Mar 15 13:38:13 2017
> @@ -2104,6 +2104,12 @@ bool ARMConstantIslands::optimizeThumb2J
>       IdxReg = Shift->getOperand(2).getReg();
>       unsigned ShiftedIdxReg = Shift->getOperand(0).getReg();
> 
> +      // It's important that IdxReg is live until the actual TBB/TBH. Most of
> +      // the range is checked later, but the LEA might still clobber it and not
> +      // actually get removed.
> +      if (BaseReg == IdxReg && !jumpTableFollowsTB(MI, User.CPEMI))
> +        continue;
> +
>       MachineInstr *Load = User.MI->getNextNode();
>       if (Load->getOpcode() != ARM::tLDRr)
>         continue;
> @@ -2135,14 +2141,14 @@ bool ARMConstantIslands::optimizeThumb2J
>           // IdxReg gets redefined in the middle of the sequence.
>           continue;
>       }
> -      
> +
>       // Now safe to delete the load and lsl. The LEA will be removed later.
>       CanDeleteLEA = true;
>       Shift->eraseFromParent();
>       Load->eraseFromParent();
>       DeadSize += 4;
>     }
> -    
> +
>     DEBUG(dbgs() << "Shrink JT: " << *MI);
>     MachineInstr *CPEMI = User.CPEMI;
>     unsigned Opc = ByteOk ? ARM::t2TBB_JT : ARM::t2TBH_JT;
> 
> Added: llvm/trunk/test/CodeGen/ARM/v6-jumptable-clobber.mir
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/ARM/v6-jumptable-clobber.mir?rev=297871&view=auto
> ==============================================================================
> --- llvm/trunk/test/CodeGen/ARM/v6-jumptable-clobber.mir (added)
> +++ llvm/trunk/test/CodeGen/ARM/v6-jumptable-clobber.mir Wed Mar 15 13:38:13 2017
> @@ -0,0 +1,384 @@
> +# RUN: llc -run-pass=arm-cp-islands -o - %s | FileCheck %s
> +
> +# Test created by tweaking the register allocation after stopping the IR below
> +# just before constant islands. We were forwarding the table index to the end of
> +# the block, even though the LEA clobbered it.
> +
> +# CHECK-LABEL: name: foo
> +# CHECK:     tBR_JT
> +  # This order is important. If the jump-table comes first then the
> +  # transformation is valid because the LEA can be removed, see second test.
> +# CHECK:     CONSTPOOL_ENTRY
> +# CHECK:     JUMPTABLE_ADDRS
> +
> +# CHECK-LABEL: name: bar
> +# CHECK: tTBB_JT %pc, killed %r1
> +
> +--- |
> +  ; ModuleID = 'simple.ll'
> +  source_filename = "simple.ll"
> +  target datalayout = "e-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64"
> +  target triple = "thumbv6m-none--eabi"
> +  
> +  define void @foo(i8 %in, i32* %addr) {
> +    store i32 12345678, i32* %addr
> +    %1 = call i32 @llvm.arm.space(i32 980, i32 undef)
> +    %2 = zext i8 %in to i32
> +    switch i32 %2, label %default [
> +      i32 0, label %d1
> +      i32 1, label %d2
> +      i32 3, label %d3
> +      i32 4, label %d4
> +      i32 5, label %d5
> +      i32 6, label %d6
> +      i32 7, label %d7
> +      i32 2, label %d8
> +      i32 8, label %d9
> +      i32 9, label %d10
> +      i32 19, label %d11
> +      i32 20, label %d12
> +      i32 21, label %d13
> +      i32 22, label %d14
> +      i32 24, label %d15
> +      i32 25, label %d16
> +      i32 26, label %d17
> +    ]
> +  
> +  default:                                          ; preds = %0
> +    unreachable
> +  
> +  d1:                                               ; preds = %0
> +    unreachable
> +  
> +  d2:                                               ; preds = %0
> +    unreachable
> +  
> +  d3:                                               ; preds = %0
> +    unreachable
> +  
> +  d4:                                               ; preds = %0
> +    unreachable
> +  
> +  d5:                                               ; preds = %0
> +    unreachable
> +  
> +  d6:                                               ; preds = %0
> +    unreachable
> +  
> +  d7:                                               ; preds = %0
> +    unreachable
> +  
> +  d8:                                               ; preds = %0
> +    unreachable
> +  
> +  d9:                                               ; preds = %0
> +    unreachable
> +  
> +  d10:                                              ; preds = %0
> +    unreachable
> +  
> +  d11:                                              ; preds = %0
> +    unreachable
> +  
> +  d12:                                              ; preds = %0
> +    unreachable
> +  
> +  d13:                                              ; preds = %0
> +    unreachable
> +  
> +  d14:                                              ; preds = %0
> +    unreachable
> +  
> +  d15:                                              ; preds = %0
> +    unreachable
> +  
> +  d16:                                              ; preds = %0
> +    unreachable
> +  
> +  d17:                                              ; preds = %0
> +    unreachable
> +  }
> +
> +  define void @bar(i8 %in, i32* %addr) {
> +      store i32 12345678, i32* %addr
> +    %1 = zext i8 %in to i32
> +    switch i32 %1, label %default [
> +      i32 0, label %d1
> +      i32 1, label %d2
> +      i32 3, label %d3
> +      i32 4, label %d4
> +      i32 5, label %d5
> +      i32 6, label %d6
> +      i32 7, label %d7
> +      i32 2, label %d8
> +      i32 8, label %d9
> +      i32 9, label %d10
> +      i32 19, label %d11
> +      i32 20, label %d12
> +      i32 21, label %d13
> +      i32 22, label %d14
> +      i32 24, label %d15
> +      i32 25, label %d16
> +      i32 26, label %d17
> +    ]
> +  
> +  default:                                          ; preds = %0
> +    unreachable
> +  
> +  d1:                                               ; preds = %0
> +    unreachable
> +  
> +  d2:                                               ; preds = %0
> +    unreachable
> +  
> +  d3:                                               ; preds = %0
> +    unreachable
> +  
> +  d4:                                               ; preds = %0
> +    unreachable
> +  
> +  d5:                                               ; preds = %0
> +    unreachable
> +  
> +  d6:                                               ; preds = %0
> +    unreachable
> +  
> +  d7:                                               ; preds = %0
> +    unreachable
> +  
> +  d8:                                               ; preds = %0
> +    unreachable
> +  
> +  d9:                                               ; preds = %0
> +    unreachable
> +  
> +  d10:                                              ; preds = %0
> +    unreachable
> +  
> +  d11:                                              ; preds = %0
> +    unreachable
> +  
> +  d12:                                              ; preds = %0
> +    unreachable
> +  
> +  d13:                                              ; preds = %0
> +    unreachable
> +  
> +  d14:                                              ; preds = %0
> +    unreachable
> +  
> +  d15:                                              ; preds = %0
> +    unreachable
> +  
> +  d16:                                              ; preds = %0
> +    unreachable
> +  
> +  d17:                                              ; preds = %0
> +    unreachable
> +  }
> +  
> +  ; Function Attrs: nounwind
> +  declare i32 @llvm.arm.space(i32, i32) #0
> +  
> +  ; Function Attrs: nounwind
> +  declare void @llvm.stackprotector(i8*, i8**) #0
> +  
> +  attributes #0 = { nounwind }
> +
> +...
> +---
> +name:            foo
> +alignment:       1
> +exposesReturnsTwice: false
> +noVRegs:         true
> +legalized:       false
> +regBankSelected: false
> +selected:        false
> +tracksRegLiveness: true
> +liveins:         
> +  - { reg: '%r0' }
> +  - { reg: '%r1' }
> +frameInfo:       
> +  isFrameAddressTaken: false
> +  isReturnAddressTaken: false
> +  hasStackMap:     false
> +  hasPatchPoint:   false
> +  stackSize:       0
> +  offsetAdjustment: 0
> +  maxAlignment:    0
> +  adjustsStack:    false
> +  hasCalls:        false
> +  maxCallFrameSize: 0
> +  hasOpaqueSPAdjustment: false
> +  hasVAStart:      false
> +  hasMustTailInVarArgFunc: false
> +constants:       
> +  - id:              0
> +    value:           i32 12345678
> +    alignment:       4
> +jumpTable:       
> +  kind:            inline
> +  entries:         
> +    - id:              0
> +      blocks:          [ '%bb.3.d2', '%bb.9.d8', '%bb.4.d3', '%bb.5.d4', 
> +                         '%bb.6.d5', '%bb.7.d6', '%bb.8.d7', '%bb.10.d9', 
> +                         '%bb.11.d10', '%bb.2.d1', '%bb.2.d1', '%bb.2.d1', 
> +                         '%bb.2.d1', '%bb.2.d1', '%bb.2.d1', '%bb.2.d1', 
> +                         '%bb.2.d1', '%bb.2.d1', '%bb.12.d11', '%bb.13.d12', 
> +                         '%bb.14.d13', '%bb.15.d14', '%bb.2.d1', '%bb.16.d15', 
> +                         '%bb.17.d16', '%bb.18.d17' ]
> +body:             |
> +  bb.0 (%ir-block.0):
> +    successors: %bb.2.d1(0x03c3c3c4), %bb.1(0x7c3c3c3c)
> +    liveins: %r0, %r1
> +  
> +    %r2 = tLDRpci %const.0, 14, _
> +    tSTRi killed %r2, killed %r1, 0, 14, _ :: (store 4 into %ir.addr)
> +    dead %r1 = SPACE 980, undef %r0
> +    %r0 = tUXTB killed %r0, 14, _
> +    %r1, dead %cpsr = tSUBi3 killed %r0, 1, 14, _
> +    tCMPi8 %r1, 25, 14, _, implicit-def %cpsr
> +    tBcc %bb.2.d1, 8, killed %cpsr
> +  
> +  bb.1 (%ir-block.0):
> +    successors: %bb.3.d2(0x07c549d2), %bb.9.d8(0x07c549d2), %bb.4.d3(0x07c549d2), %bb.5.d4(0x07c549d2), %bb.6.d5(0x07c549d2), %bb.7.d6(0x07c549d2), %bb.8.d7(0x07c549d2), %bb.10.d9(0x07c549d2), %bb.11.d10(0x07c549d2), %bb.2.d1(0x03ab62db), %bb.12.d11(0x07c549d2), %bb.13.d12(0x07c549d2), %bb.14.d13(0x07c549d2), %bb.15.d14(0x07c549d2), %bb.16.d15(0x07c549d2), %bb.17.d16(0x07c549d2), %bb.18.d17(0x07c549d2)
> +    liveins: %r1
> +  
> +    %r0, dead %cpsr = tLSLri killed %r1, 2, 14, _
> +    %r1 = tLEApcrelJT %jump-table.0, 14, _
> +    %r0 = tLDRr killed %r0, killed %r1, 14, _ :: (load 4 from jump-table)
> +    tBR_JTr killed %r0, %jump-table.0
> +  
> +  bb.3.d2:
> +  
> +  bb.9.d8:
> +  
> +  bb.4.d3:
> +  
> +  bb.5.d4:
> +  
> +  bb.6.d5:
> +  
> +  bb.7.d6:
> +  
> +  bb.8.d7:
> +  
> +  bb.10.d9:
> +  
> +  bb.11.d10:
> +  
> +  bb.2.d1:
> +  
> +  bb.12.d11:
> +  
> +  bb.13.d12:
> +  
> +  bb.14.d13:
> +  
> +  bb.15.d14:
> +  
> +  bb.16.d15:
> +  
> +  bb.17.d16:
> +  
> +  bb.18.d17:
> +
> +...
> +
> +---
> +name:            bar
> +alignment:       1
> +exposesReturnsTwice: false
> +noVRegs:         true
> +legalized:       false
> +regBankSelected: false
> +selected:        false
> +tracksRegLiveness: true
> +liveins:         
> +  - { reg: '%r0' }
> +  - { reg: '%r1' }
> +frameInfo:       
> +  isFrameAddressTaken: false
> +  isReturnAddressTaken: false
> +  hasStackMap:     false
> +  hasPatchPoint:   false
> +  stackSize:       0
> +  offsetAdjustment: 0
> +  maxAlignment:    0
> +  adjustsStack:    false
> +  hasCalls:        false
> +  maxCallFrameSize: 0
> +  hasOpaqueSPAdjustment: false
> +  hasVAStart:      false
> +  hasMustTailInVarArgFunc: false
> +constants:       
> +  - id:              0
> +    value:           i32 12345678
> +    alignment:       4
> +jumpTable:       
> +  kind:            inline
> +  entries:         
> +    - id:              0
> +      blocks:          [ '%bb.3.d2', '%bb.9.d8', '%bb.4.d3', '%bb.5.d4', 
> +                         '%bb.6.d5', '%bb.7.d6', '%bb.8.d7', '%bb.10.d9', 
> +                         '%bb.11.d10', '%bb.2.d1', '%bb.2.d1', '%bb.2.d1', 
> +                         '%bb.2.d1', '%bb.2.d1', '%bb.2.d1', '%bb.2.d1', 
> +                         '%bb.2.d1', '%bb.2.d1', '%bb.12.d11', '%bb.13.d12', 
> +                         '%bb.14.d13', '%bb.15.d14', '%bb.2.d1', '%bb.16.d15', 
> +                         '%bb.17.d16', '%bb.18.d17' ]
> +body:             |
> +  bb.0 (%ir-block.0):
> +    successors: %bb.2.d1(0x03c3c3c4), %bb.1(0x7c3c3c3c)
> +    liveins: %r0, %r1
> +  
> +    %r2 = tLDRpci %const.0, 14, _
> +    tSTRi killed %r2, killed %r1, 0, 14, _ :: (store 4 into %ir.addr)
> +    %r0 = tUXTB killed %r0, 14, _
> +    %r1, dead %cpsr = tSUBi3 killed %r0, 1, 14, _
> +    tCMPi8 %r1, 25, 14, _, implicit-def %cpsr
> +    tBcc %bb.2.d1, 8, killed %cpsr
> +  
> +  bb.1 (%ir-block.0):
> +    successors: %bb.3.d2(0x07c549d2), %bb.9.d8(0x07c549d2), %bb.4.d3(0x07c549d2), %bb.5.d4(0x07c549d2), %bb.6.d5(0x07c549d2), %bb.7.d6(0x07c549d2), %bb.8.d7(0x07c549d2), %bb.10.d9(0x07c549d2), %bb.11.d10(0x07c549d2), %bb.2.d1(0x03ab62db), %bb.12.d11(0x07c549d2), %bb.13.d12(0x07c549d2), %bb.14.d13(0x07c549d2), %bb.15.d14(0x07c549d2), %bb.16.d15(0x07c549d2), %bb.17.d16(0x07c549d2), %bb.18.d17(0x07c549d2)
> +    liveins: %r1
> +  
> +    %r0, dead %cpsr = tLSLri killed %r1, 2, 14, _
> +    %r1 = tLEApcrelJT %jump-table.0, 14, _
> +    %r0 = tLDRr killed %r0, killed %r1, 14, _ :: (load 4 from jump-table)
> +    tBR_JTr killed %r0, %jump-table.0
> +  
> +  bb.3.d2:
> +  
> +  bb.9.d8:
> +  
> +  bb.4.d3:
> +  
> +  bb.5.d4:
> +  
> +  bb.6.d5:
> +  
> +  bb.7.d6:
> +  
> +  bb.8.d7:
> +  
> +  bb.10.d9:
> +  
> +  bb.11.d10:
> +  
> +  bb.2.d1:
> +  
> +  bb.12.d11:
> +  
> +  bb.13.d12:
> +  
> +  bb.14.d13:
> +  
> +  bb.15.d14:
> +  
> +  bb.16.d15:
> +  
> +  bb.17.d16:
> +  
> +  bb.18.d17:
> +
> +...
> 
> Modified: llvm/trunk/test/CodeGen/X86/GlobalISel/irtranslator-call.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/GlobalISel/irtranslator-call.ll?rev=297871&r1=297870&r2=297871&view=diff
> ==============================================================================
> --- llvm/trunk/test/CodeGen/X86/GlobalISel/irtranslator-call.ll (original)
> +++ llvm/trunk/test/CodeGen/X86/GlobalISel/irtranslator-call.ll Wed Mar 15 13:38:13 2017
> @@ -5,6 +5,7 @@ define void @test_void_return() {
> ; CHECK-LABEL: name:            test_void_return
> ; CHECK:      alignment:       4
> ; CHECK-NEXT: exposesReturnsTwice: false
> +; CHECK-NEXT: noVRegs:         false
> ; CHECK-NEXT: legalized:       false
> ; CHECK-NEXT: regBankSelected: false
> ; CHECK-NEXT: selected:        false
> 
> 
> _______________________________________________
> 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