[llvm] r242359 - Clear kill flags in ARMLoadStoreOptimizer.

[Duncan P. N. Exon Smith]

> On 2015-Jul-15, at 17:09, Pete Cooper <peter_cooper at apple.com> wrote:
> 
> Author: pete
> Date: Wed Jul 15 19:09:18 2015
> New Revision: 242359
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=242359&view=rev
> Log:
> Clear kill flags in ARMLoadStoreOptimizer.
> 
> The pass here was clearing kill flags on instructions which had
> their sources killed in the instruction being combined.  But
> given that the new instruction is inserted after the existing ones,
> any existing instructions with kill flags will lead to the verifier
> complaining that we are reading an undefined physreg.
> 
> For example, what we had prior to this optimization is
> 	t2STRi12 %R1, %SP, 12
> 	t2STRi12 %R1<kill>, %SP, 16
> 	t2STRi12 %R0<kill>, %SP, 8
> 
> and prior to this fix that would generate
> 	t2STRi12 %R1<kill>, %SP, 16
> 	t2STRDi8 %R0<kill>, %R1, %SP, 8
> 
> This is clearly incorrect as it didn't clear the kill flag on R1
> used with offset 16 because there was no kill flag on the instruction
> with offset 12.
> 
> After this change we clear the kill flag on the offset 16 instruction
> because we know it will be used afterwards in the new instruction.
> 
> I haven't provided a test case.  I have a small test, but even it is
> very sensitive to register allocation order which isn't ideal.

Alex, is MIR serialization far enough along that this can be tested
now?

> 
> Modified:
>    llvm/trunk/lib/Target/ARM/ARMLoadStoreOptimizer.cpp
> 
> Modified: llvm/trunk/lib/Target/ARM/ARMLoadStoreOptimizer.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMLoadStoreOptimizer.cpp?rev=242359&r1=242358&r2=242359&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/ARM/ARMLoadStoreOptimizer.cpp (original)
> +++ llvm/trunk/lib/Target/ARM/ARMLoadStoreOptimizer.cpp Wed Jul 15 19:09:18 2015
> @@ -786,6 +786,7 @@ MachineInstr *ARMLoadStoreOpt::MergeOpsU
>   SmallVector<std::pair<unsigned, bool>, 8> Regs;
>   SmallVector<unsigned, 4> ImpDefs;
>   DenseSet<unsigned> KilledRegs;
> +  DenseSet<unsigned> UsedRegs;
>   // Determine list of registers and list of implicit super-register defs.
>   for (const MachineInstr *MI : Cand.Instrs) {
>     const MachineOperand &MO = getLoadStoreRegOp(*MI);
> @@ -794,6 +795,7 @@ MachineInstr *ARMLoadStoreOpt::MergeOpsU
>     if (IsKill)
>       KilledRegs.insert(Reg);
>     Regs.push_back(std::make_pair(Reg, IsKill));
> +    UsedRegs.insert(Reg);
> 
>     if (IsLoad) {
>       // Collect any implicit defs of super-registers, after merging we can't
> @@ -883,7 +885,7 @@ MachineInstr *ARMLoadStoreOpt::MergeOpsU
>       for (MachineOperand &MO : MI.uses()) {
>         if (!MO.isReg() || !MO.isKill())
>           continue;
> -        if (KilledRegs.count(MO.getReg()))
> +        if (UsedRegs.count(MO.getReg()))
>           MO.setIsKill(false);
>       }
>     }
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits