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

[Matthias Braun via llvm-commits]

Luckily AArch64 does not use any bundles so no issue here either way.

As for bundles: We appear to have 2 bundle modelings in llvm:
- One variant is having a BUNDLE instruction in front of the bundle which repeats the relevant operands so the normal code will get the relevant information just by looking at the first instruction without the need to look what's actually inside the bundle.
- The other variant has no BUNDLE instruction and forces code to use stuff like the MIBundleOperands from MachineInstrBundle,h
I still don't know which variant is supposed to be used when. 

- Matthias

> On Aug 18, 2016, at 7:29 PM, Duncan P. N. Exon Smith via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
>> 
>> 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
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits <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/20160818/d61fa452/attachment.html>