[llvm] r279104 - AArch64: Don't call getIterator() on iterators

[Duncan P. N. Exon Smith via llvm-commits]

> On 2016-Aug-18, at 10:58, Duncan P. N. Exon Smith via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
> Author: dexonsmith
> Date: Thu Aug 18 12:58:09 2016
> New Revision: 279104
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=279104&view=rev
> Log:
> AArch64: Don't call getIterator() on iterators
> 
> Remove an unnecessary round-trip:
> 
>    iterator => operator->() => getIterator()
> 
> In some cases, the iterator is end(), so the dereference of operator->
> is invalid (UB).
> 
> The testcase only crashes with r278974 (currently reverted to
> investigate this), which adds an assertion for invalid dereferences of
> ilist nodes.
> 
> Fixes PR29035.
> 
> Added:
>    llvm/trunk/test/CodeGen/AArch64/redundant-copy-elim-empty-mbb.ll
> Modified:
>    llvm/trunk/lib/Target/AArch64/AArch64RedundantCopyElimination.cpp
> 
> Modified: llvm/trunk/lib/Target/AArch64/AArch64RedundantCopyElimination.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64RedundantCopyElimination.cpp?rev=279104&r1=279103&r2=279104&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/AArch64/AArch64RedundantCopyElimination.cpp (original)
> +++ llvm/trunk/lib/Target/AArch64/AArch64RedundantCopyElimination.cpp Thu Aug 18 12:58:09 2016
> @@ -156,8 +156,7 @@ bool AArch64RedundantCopyElimination::op
>     MBB->addLiveIn(TargetReg);
> 
>   // Clear any kills of TargetReg between CompBr and the last removed COPY.
> -  for (MachineInstr &MMI :
> -       make_range(MBB->begin()->getIterator(), LastChange->getIterator()))
> +  for (MachineInstr &MMI : make_range(MBB->begin(), LastChange))

After-thought: does this do the right thing for bundled instructions?

I.e., should this code be, instead:

    for (MachineInstr &MMI : make_range(MBB->begin().getInstrIterator(),
                                        LastChange.getInstrIterator()))

This matches the old semantics (in the case where there wasn't UB).

>     MMI.clearRegisterKills(SmallestDef, TRI);
> 
>   return true;
> 
> Added: llvm/trunk/test/CodeGen/AArch64/redundant-copy-elim-empty-mbb.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AArch64/redundant-copy-elim-empty-mbb.ll?rev=279104&view=auto
> ==============================================================================
> --- llvm/trunk/test/CodeGen/AArch64/redundant-copy-elim-empty-mbb.ll (added)
> +++ llvm/trunk/test/CodeGen/AArch64/redundant-copy-elim-empty-mbb.ll Thu Aug 18 12:58:09 2016
> @@ -0,0 +1,29 @@
> +; RUN: llc < %s | FileCheck %s
> +; Make sure we don't crash in AArch64RedundantCopyElimination when a
> +; MachineBasicBlock is empty.  PR29035.
> +
> +target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
> +target triple = "aarch64-unknown-linux-gnu"
> +
> +declare i8* @bar()
> +
> +; CHECK-LABEL: foo:
> +; CHECK: tbz
> +; CHECK: orr
> +; CHECK: ret
> +; CHECK: bl bar
> +; CHECK: cbnz
> +; CHECK: ret
> +define i1 @foo(i1 %start) {
> +entry:
> +  br i1 %start, label %cleanup, label %if.end
> +
> +if.end:                                           ; preds = %if.end, %entry
> +  %call = tail call i8* @bar()
> +  %cmp = icmp eq i8* %call, null
> +  br i1 %cmp, label %cleanup, label %if.end
> +
> +cleanup:                                          ; preds = %if.end, %entry
> +  %retval.0 = phi i1 [ true, %entry ], [ false, %if.end ]
> +  ret i1 %retval.0
> +}
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits