[llvm] r209242 - [ARM64] PR19792: Fix cycle in DAG after performPostLD1Combine
David Blaikie
dblaikie at gmail.com
Tue May 20 16:07:21 PDT 2014
As an aside, tests that simplify verify that something doesn't crash
are usually a bit of a code smell from my perspective.
If the code previously crashed - what was the missing behavioral test
case? There must be some behavior that was intended (other than "don't
crash") that we weren't testing for and should be now testing for...
On Tue, May 20, 2014 at 3:52 PM, Adam Nemet <anemet at apple.com> wrote:
> On May 20, 2014, at 3:49 PM, Alexey Samsonov <samsonov at google.com> wrote:
>
>
> On Tue, May 20, 2014 at 2:47 PM, Adam Nemet <anemet at apple.com> wrote:
>>
>> Author: anemet
>> Date: Tue May 20 16:47:07 2014
>> New Revision: 209242
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=209242&view=rev
>> Log:
>> [ARM64] PR19792: Fix cycle in DAG after performPostLD1Combine
>>
>> Povray and dealII currently assert with "Overran sorted position" in
>> AssignTopologicalOrder. The problem is that performPostLD1Combine can
>> introduce cycles.
>>
>> Consider:
>>
>> (insert_vector_elt (INSERT_SUBREG undef,
>> (load (add %vreg0, Constant<8>), undef),
>> <= A
>> TargetConstant<2>),
>> (load %vreg0, undef),
>> <= B
>> Constant<1>)
>>
>> This is turned into a LD1LANEpost node. However the address in A is not a
>> valid user of the post-incremented address of B in LD1LANEpost.
>>
>> Added:
>> llvm/trunk/test/CodeGen/ARM64/indexed-vector-ldst-2.ll
>> Modified:
>> llvm/trunk/lib/Target/ARM64/ARM64ISelLowering.cpp
>>
>> Modified: llvm/trunk/lib/Target/ARM64/ARM64ISelLowering.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM64/ARM64ISelLowering.cpp?rev=209242&r1=209241&r2=209242&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/Target/ARM64/ARM64ISelLowering.cpp (original)
>> +++ llvm/trunk/lib/Target/ARM64/ARM64ISelLowering.cpp Tue May 20 16:47:07
>> 2014
>> @@ -7298,6 +7298,7 @@ static SDValue performPostLD1Combine(SDN
>> }
>>
>> SDValue Addr = LD->getOperand(1);
>> + SDValue Vector = N->getOperand(0);
>> // Search for a use of the address operand that is an increment.
>> for (SDNode::use_iterator UI = Addr.getNode()->use_begin(), UE =
>> Addr.getNode()->use_end(); UI != UE; ++UI) {
>> @@ -7310,6 +7311,10 @@ static SDValue performPostLD1Combine(SDN
>> // would create a cycle.
>> if (User->isPredecessorOf(LD) || LD->isPredecessorOf(User))
>> continue;
>> + // Also check that add is not used in the vector operand. This would
>> also
>> + // create a cycle.
>> + if (User->isPredecessorOf(Vector.getNode()))
>> + continue;
>>
>> // If the increment is a constant, it must match the memory ref size.
>> SDValue Inc = User->getOperand(User->getOperand(0) == Addr ? 1 : 0);
>> @@ -7324,7 +7329,7 @@ static SDValue performPostLD1Combine(SDN
>> SmallVector<SDValue, 8> Ops;
>> Ops.push_back(LD->getOperand(0)); // Chain
>> if (IsLaneOp) {
>> - Ops.push_back(N->getOperand(0)); // The vector to be inserted
>> + Ops.push_back(Vector); // The vector to be inserted
>> Ops.push_back(N->getOperand(2)); // The lane to be inserted in the
>> vector
>> }
>> Ops.push_back(Addr);
>>
>> Added: llvm/trunk/test/CodeGen/ARM64/indexed-vector-ldst-2.ll
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/ARM64/indexed-vector-ldst-2.ll?rev=209242&view=auto
>>
>> ==============================================================================
>> --- llvm/trunk/test/CodeGen/ARM64/indexed-vector-ldst-2.ll (added)
>> +++ llvm/trunk/test/CodeGen/ARM64/indexed-vector-ldst-2.ll Tue May 20
>> 16:47:07 2014
>> @@ -0,0 +1,40 @@
>> +; RUN: llc %s
>
>
> ^^
> This line would create file test/CodeGen/ARM64/indexed-vector-ldst-2.s in
> the source tree. I've mailed a fix in r209252.
>
>
> Ah! Sorry about that and thanks for the fix!
>
> Adam
>
>
>>
>> +
>> +; This used to assert with "Overran sorted position" in
>> AssignTopologicalOrder
>> +; due to a cycle created in performPostLD1Combine.
>> +
>> +target datalayout = "e-m:o-i64:64-i128:128-n32:64-S128"
>> +target triple = "arm64-apple-ios7.0.0"
>> +
>> +; Function Attrs: nounwind ssp
>> +define void @f(double* %P1) #0 {
>> +entry:
>> + %arrayidx4 = getelementptr inbounds double* %P1, i64 1
>> + %0 = load double* %arrayidx4, align 8, !tbaa !1
>> + %1 = load double* %P1, align 8, !tbaa !1
>> + %2 = insertelement <2 x double> undef, double %0, i32 0
>> + %3 = insertelement <2 x double> %2, double %1, i32 1
>> + %4 = fsub <2 x double> zeroinitializer, %3
>> + %5 = fmul <2 x double> undef, %4
>> + %6 = extractelement <2 x double> %5, i32 0
>> + %cmp168 = fcmp olt double %6, undef
>> + br i1 %cmp168, label %if.then172, label %return
>> +
>> +if.then172: ; preds = %cond.end90
>> + %7 = tail call i64 @llvm.objectsize.i64.p0i8(i8* undef, i1 false)
>> + br label %return
>> +
>> +return: ; preds = %if.then172,
>> %cond.end90, %entry
>> + ret void
>> +}
>> +
>> +; Function Attrs: nounwind readnone
>> +declare i64 @llvm.objectsize.i64.p0i8(i8*, i1) #1
>> +
>> +attributes #0 = { nounwind ssp "less-precise-fpmad"="false"
>> "no-frame-pointer-elim"="false" "no-infs-fp-math"="false"
>> "no-nans-fp-math"="false" "stack-protector-buffer-size"="8"
>> "unsafe-fp-math"="false" "use-soft-float"="false" }
>> +attributes #1 = { nounwind readnone }
>> +
>> +!1 = metadata !{metadata !2, metadata !2, i64 0}
>> +!2 = metadata !{metadata !"double", metadata !3, i64 0}
>> +!3 = metadata !{metadata !"omnipotent char", metadata !4, i64 0}
>> +!4 = metadata !{metadata !"Simple C/C++ TBAA"}
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
>
>
> --
> Alexey Samsonov, Mountain View, CA
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
More information about the llvm-commits
mailing list