Skip to content

Commit cbab9a5

Browse files
mshockwavemshockwave
committed
Address review comments
And checks whether the tree is balanced.
1 parent 735ce76 commit cbab9a5

File tree

2 files changed

+133
-16
lines changed

2 files changed

+133
-16
lines changed

‎llvm/lib/CodeGen/InterleavedAccessPass.cpp

Lines changed: 26 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -257,7 +257,7 @@ static bool isReInterleaveMask(ShuffleVectorInst *SVI, unsigned &Factor,
257257
// |
258258
// A B C D
259259
//
260-
// We will get ABCD at the end while the leave operands/results
260+
// We will get ABCD at the end while the leaf operands/results
261261
// are ACBD, which are also what we initially collected in
262262
// getVectorInterleaveFactor / getVectorDeinterleaveFactor. But TLI
263263
// hooks (e.g. lowerInterleavedScalableLoad) expect ABCD, so we need
@@ -311,18 +311,23 @@ static unsigned getVectorInterleaveFactor(IntrinsicInst *II,
311311
continue;
312312
}
313313

314+
// If this is not a perfectly balanced tree, the leaf
315+
// result types would be different.
316+
if (!Operands.empty() && Op->getType() != Operands.back()->getType())
317+
return 0;
318+
314319
++Factor;
315320
Operands.push_back(Op);
316321
}
317322
}
318323

319324
// Currently we only recognize power-of-two factors.
320325
// FIXME: should we assert here instead?
321-
if (Factor > 1 && isPowerOf2_32(Factor)) {
322-
interleaveLeafValues(Operands);
323-
return Factor;
324-
}
325-
return 0;
326+
if (Factor <= 1 || !isPowerOf2_32(Factor))
327+
return 0;
328+
329+
interleaveLeafValues(Operands);
330+
return Factor;
326331
}
327332

328333
/// Check the interleaved mask
@@ -367,7 +372,7 @@ static unsigned getVectorDeInterleaveFactor(IntrinsicInst *II,
367372

368373
unsigned VisitedIdx = 0;
369374
for (User *Usr : Current->users()) {
370-
// We're playing safe here and matches only the expression
375+
// We're playing safe here and matching only the expression
371376
// consisting of a perfectly balanced binary tree in which all
372377
// intermediate values are only used once.
373378
if (!Usr->hasOneUse() || !isa<ExtractValueInst>(Usr))
@@ -379,10 +384,10 @@ static unsigned getVectorDeInterleaveFactor(IntrinsicInst *II,
379384
return 0;
380385

381386
// The idea is that we don't want to have two extractvalue
382-
// on the same index. So we XOR (index + 1) onto VisitedIdx
387+
// on the same index. So we XOR (1 << index) onto VisitedIdx
383388
// such that if there is any duplication, VisitedIdx will be
384389
// zero.
385-
VisitedIdx ^= Indices[0] + 1;
390+
VisitedIdx ^= (1 << Indices[0]);
386391
if (!VisitedIdx)
387392
return 0;
388393
// We have a legal index. At this point we're either going
@@ -403,15 +408,20 @@ static unsigned getVectorDeInterleaveFactor(IntrinsicInst *II,
403408
m_Intrinsic<Intrinsic::vector_deinterleave2>()) &&
404409
EV->user_back()->hasNUses(2)) {
405410
auto *EVUsr = cast<IntrinsicInst>(EV->user_back());
406-
if (SwapWithLast)
411+
if (SwapWithLast && !Queue.empty())
407412
Queue.insert(Queue.end() - 1, EVUsr);
408413
else
409414
Queue.push_back(EVUsr);
410415
continue;
411416
}
412417

418+
// If this is not a perfectly balanced tree, the leaf
419+
// result types would be different.
420+
if (!Results.empty() && EV->getType() != Results.back()->getType())
421+
return 0;
422+
413423
// Save the leaf value.
414-
if (SwapWithLast)
424+
if (SwapWithLast && !Results.empty())
415425
Results.insert(Results.end() - 1, EV);
416426
else
417427
Results.push_back(EV);
@@ -422,11 +432,11 @@ static unsigned getVectorDeInterleaveFactor(IntrinsicInst *II,
422432

423433
// Currently we only recognize power-of-two factors.
424434
// FIXME: should we assert here instead?
425-
if (Factor > 1 && isPowerOf2_32(Factor)) {
426-
interleaveLeafValues(Results);
427-
return Factor;
428-
}
429-
return 0;
435+
if (Factor <= 1 || !isPowerOf2_32(Factor))
436+
return 0;
437+
438+
interleaveLeafValues(Results);
439+
return Factor;
430440
}
431441

432442
bool InterleavedAccessImpl::lowerInterleavedLoad(

‎llvm/test/CodeGen/RISCV/rvv/scalable-vectors-interleaved-access.ll

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -507,6 +507,113 @@ define {<vscale x 2 x i32>, <vscale x 2 x i32>, <vscale x 2 x i32>, <vscale x 2
507507
ret { <vscale x 2 x i32>, <vscale x 2 x i32>, <vscale x 2 x i32>, <vscale x 2 x i32> } %res3
508508
}
509509

510+
; We should not transform this function because the expression is not a balanced tree.
511+
define {<vscale x 4 x i32>, <vscale x 2 x i32>, <vscale x 1 x i32>, <vscale x 1 x i32>} @not_balanced_load_tree(ptr %ptr, i32 %rvl) {
512+
; RV32-LABEL: not_balanced_load_tree:
513+
; RV32: # %bb.0:
514+
; RV32-NEXT: vsetvli zero, a1, e32, m4, ta, ma
515+
; RV32-NEXT: vle32.v v12, (a0)
516+
; RV32-NEXT: li a0, 32
517+
; RV32-NEXT: vsetvli a1, zero, e32, m2, ta, ma
518+
; RV32-NEXT: vnsrl.wx v8, v12, a0
519+
; RV32-NEXT: vnsrl.wi v16, v12, 0
520+
; RV32-NEXT: vsetvli a1, zero, e32, m1, ta, ma
521+
; RV32-NEXT: vnsrl.wi v10, v16, 0
522+
; RV32-NEXT: vnsrl.wx v11, v16, a0
523+
; RV32-NEXT: vsetvli a1, zero, e32, mf2, ta, ma
524+
; RV32-NEXT: vnsrl.wx v12, v11, a0
525+
; RV32-NEXT: vnsrl.wi v11, v11, 0
526+
; RV32-NEXT: ret
527+
;
528+
; RV64-LABEL: not_balanced_load_tree:
529+
; RV64: # %bb.0:
530+
; RV64-NEXT: slli a1, a1, 32
531+
; RV64-NEXT: srli a1, a1, 32
532+
; RV64-NEXT: vsetvli zero, a1, e32, m4, ta, ma
533+
; RV64-NEXT: vle32.v v12, (a0)
534+
; RV64-NEXT: li a0, 32
535+
; RV64-NEXT: vsetvli a1, zero, e32, m2, ta, ma
536+
; RV64-NEXT: vnsrl.wx v8, v12, a0
537+
; RV64-NEXT: vnsrl.wi v16, v12, 0
538+
; RV64-NEXT: vsetvli a1, zero, e32, m1, ta, ma
539+
; RV64-NEXT: vnsrl.wi v10, v16, 0
540+
; RV64-NEXT: vnsrl.wx v11, v16, a0
541+
; RV64-NEXT: vsetvli a1, zero, e32, mf2, ta, ma
542+
; RV64-NEXT: vnsrl.wx v12, v11, a0
543+
; RV64-NEXT: vnsrl.wi v11, v11, 0
544+
; RV64-NEXT: ret
545+
%wide.masked.load = call <vscale x 8 x i32> @llvm.vp.load.nxv8i32.p0(ptr %ptr, <vscale x 8 x i1> shufflevector (<vscale x 8 x i1> insertelement (<vscale x 8 x i1> poison, i1 true, i32 0), <vscale x 8 x i1> poison, <vscale x 8 x i32> zeroinitializer), i32 %rvl)
546+
%d0 = call { <vscale x 4 x i32>, <vscale x 4 x i32> } @llvm.vector.deinterleave2.nxv8i32(<vscale x 8 x i32> %wide.masked.load)
547+
%d0.0 = extractvalue { <vscale x 4 x i32>, <vscale x 4 x i32> } %d0, 0
548+
%t0 = extractvalue { <vscale x 4 x i32>, <vscale x 4 x i32> } %d0, 1
549+
%d1 = call { <vscale x 2 x i32>, <vscale x 2 x i32> } @llvm.vector.deinterleave2.nxv4i32(<vscale x 4 x i32> %d0.0)
550+
%t1 = extractvalue { <vscale x 2 x i32>, <vscale x 2 x i32> } %d1, 0
551+
%d1.1 = extractvalue { <vscale x 2 x i32>, <vscale x 2 x i32> } %d1, 1
552+
%d2 = call { <vscale x 1 x i32>, <vscale x 1 x i32> } @llvm.vector.deinterleave2.nxv4i32(<vscale x 2 x i32> %d1.1)
553+
%t2 = extractvalue { <vscale x 1 x i32>, <vscale x 1 x i32> } %d2, 0
554+
%t3 = extractvalue { <vscale x 1 x i32>, <vscale x 1 x i32> } %d2, 1
555+
556+
%res0 = insertvalue { <vscale x 4 x i32>, <vscale x 2 x i32>, <vscale x 1 x i32>, <vscale x 1 x i32> } undef, <vscale x 4 x i32> %t0, 0
557+
%res1 = insertvalue { <vscale x 4 x i32>, <vscale x 2 x i32>, <vscale x 1 x i32>, <vscale x 1 x i32> } %res0, <vscale x 2 x i32> %t1, 1
558+
%res2 = insertvalue { <vscale x 4 x i32>, <vscale x 2 x i32>, <vscale x 1 x i32>, <vscale x 1 x i32> } %res1, <vscale x 1 x i32> %t2, 2
559+
%res3 = insertvalue { <vscale x 4 x i32>, <vscale x 2 x i32>, <vscale x 1 x i32>, <vscale x 1 x i32> } %res2, <vscale x 1 x i32> %t3, 3
560+
ret { <vscale x 4 x i32>, <vscale x 2 x i32>, <vscale x 1 x i32>, <vscale x 1 x i32> } %res3
561+
}
562+
563+
define void @not_balanced_store_tree(<vscale x 1 x i32> %v0, <vscale x 2 x i32> %v1, <vscale x 4 x i32> %v2, ptr %ptr, i32 %rvl) {
564+
; RV32-LABEL: not_balanced_store_tree:
565+
; RV32: # %bb.0:
566+
; RV32-NEXT: vsetvli a2, zero, e32, mf2, ta, ma
567+
; RV32-NEXT: vwaddu.vv v12, v8, v8
568+
; RV32-NEXT: li a2, -1
569+
; RV32-NEXT: csrr a3, vlenb
570+
; RV32-NEXT: vwmaccu.vx v12, a2, v8
571+
; RV32-NEXT: srli a3, a3, 3
572+
; RV32-NEXT: vsetvli a4, zero, e32, m1, ta, ma
573+
; RV32-NEXT: vslidedown.vx v8, v12, a3
574+
; RV32-NEXT: add a4, a3, a3
575+
; RV32-NEXT: vsetvli zero, a4, e32, m1, ta, ma
576+
; RV32-NEXT: vslideup.vx v12, v8, a3
577+
; RV32-NEXT: vsetvli a3, zero, e32, m1, ta, ma
578+
; RV32-NEXT: vwaddu.vv v14, v12, v9
579+
; RV32-NEXT: vwmaccu.vx v14, a2, v9
580+
; RV32-NEXT: vsetvli a3, zero, e32, m2, ta, ma
581+
; RV32-NEXT: vwaddu.vv v16, v14, v10
582+
; RV32-NEXT: vwmaccu.vx v16, a2, v10
583+
; RV32-NEXT: vsetvli zero, a1, e32, m4, ta, ma
584+
; RV32-NEXT: vse32.v v16, (a0)
585+
; RV32-NEXT: ret
586+
;
587+
; RV64-LABEL: not_balanced_store_tree:
588+
; RV64: # %bb.0:
589+
; RV64-NEXT: vsetvli a2, zero, e32, mf2, ta, ma
590+
; RV64-NEXT: vwaddu.vv v12, v8, v8
591+
; RV64-NEXT: li a2, -1
592+
; RV64-NEXT: csrr a3, vlenb
593+
; RV64-NEXT: slli a1, a1, 32
594+
; RV64-NEXT: vwmaccu.vx v12, a2, v8
595+
; RV64-NEXT: srli a3, a3, 3
596+
; RV64-NEXT: vsetvli a4, zero, e32, m1, ta, ma
597+
; RV64-NEXT: vslidedown.vx v8, v12, a3
598+
; RV64-NEXT: add a4, a3, a3
599+
; RV64-NEXT: vsetvli zero, a4, e32, m1, ta, ma
600+
; RV64-NEXT: vslideup.vx v12, v8, a3
601+
; RV64-NEXT: vsetvli a3, zero, e32, m1, ta, ma
602+
; RV64-NEXT: vwaddu.vv v14, v12, v9
603+
; RV64-NEXT: vwmaccu.vx v14, a2, v9
604+
; RV64-NEXT: vsetvli a3, zero, e32, m2, ta, ma
605+
; RV64-NEXT: vwaddu.vv v16, v14, v10
606+
; RV64-NEXT: vwmaccu.vx v16, a2, v10
607+
; RV64-NEXT: srli a1, a1, 32
608+
; RV64-NEXT: vsetvli zero, a1, e32, m4, ta, ma
609+
; RV64-NEXT: vse32.v v16, (a0)
610+
; RV64-NEXT: ret
611+
%interleaved.vec0 = call <vscale x 2 x i32> @llvm.vector.interleave2.nxv2i32(<vscale x 1 x i32> %v0, <vscale x 1 x i32> %v0)
612+
%interleaved.vec1 = call <vscale x 4 x i32> @llvm.vector.interleave2.nxv2i32(<vscale x 2 x i32> %interleaved.vec0, <vscale x 2 x i32> %v1)
613+
%interleaved.vec2 = call <vscale x 8 x i32> @llvm.vector.interleave2.nxv4i32(<vscale x 4 x i32> %interleaved.vec1, <vscale x 4 x i32> %v2)
614+
call void @llvm.vp.store.nxv8i32.p0(<vscale x 8 x i32> %interleaved.vec2, ptr %ptr, <vscale x 8 x i1> shufflevector (<vscale x 8 x i1> insertelement (<vscale x 8 x i1> poison, i1 true, i32 0), <vscale x 8 x i1> poison, <vscale x 8 x i32> zeroinitializer), i32 %rvl)
615+
ret void
616+
}
510617

511618
;; NOTE: These prefixes are unused and the list is autogenerated. Do not add tests below this line:
512619
; CHECK: {{.*}}

0 commit comments

Comments
 (0)