Skip to content

Commit 5dd0ad0

Browse files
sebpopsebpop
committed
fixes for Alexey's review
1 parent aa17a08 commit 5dd0ad0

File tree

1 file changed

+46
-59
lines changed

1 file changed

+46
-59
lines changed

‎llvm/lib/Analysis/Delinearization.cpp‎

Lines changed: 46 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@
1313
//
1414
//===----------------------------------------------------------------------===//
1515

16-
#include "llvm/Analysis/Delinearization.h"
1716
#include "llvm/ADT/DenseMap.h"
17+
#include "llvm/Analysis/Delinearization.h"
1818
#include "llvm/Analysis/LoopInfo.h"
1919
#include "llvm/Analysis/ScalarEvolution.h"
2020
#include "llvm/Analysis/ScalarEvolutionDivision.h"
@@ -41,7 +41,7 @@ static cl::opt<bool> UseFixedSizeArrayHeuristic(
4141
cl::desc("When printing analysis, use the heuristic for fixed-size arrays "
4242
"if the default delinearizetion fails."));
4343

44-
static cl::opt<bool> useGEPToDelinearize(
44+
static cl::opt<bool> UseGEPToDelinearize(
4545
"use-gep-to-delinearize", cl::init(true), cl::Hidden,
4646
cl::desc("validate both delinearization methods match."));
4747

@@ -623,9 +623,8 @@ void llvm::delinearize(ScalarEvolution &SE, const SCEV *Expr,
623623
LLVM_DEBUG(dbgs() << "delinearize falling back to parametric method\n");
624624

625625
// Fall back to parametric delinearization.
626-
const SCEVUnknown *BasePointer =
627-
dyn_cast<SCEVUnknown>(SE.getPointerBase(Expr));
628-
if (BasePointer)
626+
if (const SCEVUnknown *BasePointer =
627+
dyn_cast<SCEVUnknown>(SE.getPointerBase(Expr)))
629628
Expr = SE.getMinusSCEV(Expr, BasePointer);
630629

631630
SmallVector<const SCEV *, 4> Terms;
@@ -895,8 +894,8 @@ bool llvm::tryDelinearizeFixedSizeImpl(
895894
if (!SrcGEP)
896895
return false;
897896

898-
// When flag useGEPToDelinearize is false, delinearize only using array_info.
899-
if (!useGEPToDelinearize) {
897+
// When flag UseGEPToDelinearize is false, delinearize only using array_info.
898+
if (!UseGEPToDelinearize) {
900899
SmallVector<const SCEV *, 4> SCEVSizes;
901900
const SCEV *ElementSize = SE->getElementSize(Inst);
902901
if (!delinearizeUsingArrayInfo(*SE, AccessFn, Subscripts, SCEVSizes,
@@ -913,7 +912,7 @@ bool llvm::tryDelinearizeFixedSizeImpl(
913912
}
914913

915914
// TODO: Remove all the following code once we are satisfied with array_info.
916-
// Run both methods when useGEPToDelinearize is true: validation is enabled.
915+
// Run both methods when UseGEPToDelinearize is true: validation is enabled.
917916

918917
// Store results from both methods.
919918
SmallVector<const SCEV *, 4> GEPSubscripts, ArrayInfoSubscripts;
@@ -937,82 +936,70 @@ bool llvm::tryDelinearizeFixedSizeImpl(
937936
convertSCEVSizesToIntSizes(SCEVSizes, ArrayInfoSizes);
938937

939938
// Validate consistency between methods.
940-
if (GEPSuccess && ArrayInfoSuccess) {
941-
// If both methods succeeded, validate they produce the same results.
942-
// Compare sizes arrays.
943-
if (GEPSizes.size() + 2 != ArrayInfoSizes.size()) {
944-
LLVM_DEBUG({
939+
LLVM_DEBUG({
940+
if (GEPSuccess && ArrayInfoSuccess) {
941+
// If both methods succeeded, validate they produce the same results.
942+
// Compare sizes arrays.
943+
if (GEPSizes.size() + 2 != ArrayInfoSizes.size()) {
945944
dbgs() << "WARN: Size arrays have different lengths!\n";
946945
dbgs() << "GEP sizes count: " << GEPSizes.size() << "\n"
947946
<< "ArrayInfo sizes count: " << ArrayInfoSizes.size() << "\n";
948-
});
949-
}
947+
}
950948

951-
for (size_t i = 0; i < GEPSizes.size(); ++i) {
952-
if (GEPSizes[i] != ArrayInfoSizes[i + 1]) {
953-
LLVM_DEBUG({
954-
dbgs() << "WARN: Size arrays differ at index " << i << "!\n";
955-
dbgs() << "GEP size[" << i << "]: " << GEPSizes[i] << "\n"
956-
<< "ArrayInfo size[" << i + 1 << "]: " << ArrayInfoSizes[i + 1]
949+
for (size_t I : seq<size_t>(GEPSizes.size())) {
950+
if (GEPSizes[I] != ArrayInfoSizes[I + 1]) {
951+
dbgs() << "WARN: Size arrays differ at index " << I << "!\n";
952+
dbgs() << "GEP size[" << I << "]: " << GEPSizes[I] << "\n"
953+
<< "ArrayInfo size[" << I + 1 << "]: " << ArrayInfoSizes[I + 1]
957954
<< "\n";
958-
});
955+
}
959956
}
960-
}
961957

962-
// Compare subscripts arrays.
963-
if (GEPSubscripts.size() != ArrayInfoSubscripts.size()) {
964-
LLVM_DEBUG({
958+
// Compare subscripts arrays.
959+
if (GEPSubscripts.size() != ArrayInfoSubscripts.size()) {
965960
dbgs() << "WARN: Subscript arrays have different lengths!\n";
966961
dbgs() << " GEP subscripts count: " << GEPSubscripts.size() << "\n"
967962
<< " ArrayInfo subscripts count: " << ArrayInfoSubscripts.size()
968963
<< "\n";
969964

970965
dbgs() << " GEP subscripts:\n";
971-
for (size_t i = 0; i < GEPSubscripts.size(); ++i)
972-
dbgs() << " subscript[" << i << "]: " << *GEPSubscripts[i] << "\n";
966+
for (size_t I : seq<size_t>(GEPSubscripts.size()))
967+
dbgs() << " subscript[" << I << "]: " << *GEPSubscripts[I] << "\n";
973968

974969
dbgs() << " ArrayInfo subscripts:\n";
975-
for (size_t i = 0; i < ArrayInfoSubscripts.size(); ++i)
976-
dbgs() << " subscript[" << i << "]: " << *ArrayInfoSubscripts[i]
970+
for (size_t I : seq<size_t>(ArrayInfoSubscripts.size()))
971+
dbgs() << " subscript[" << I << "]: " << *ArrayInfoSubscripts[I]
977972
<< "\n";
978-
});
979-
}
973+
}
980974

981-
for (size_t i = 0; i < GEPSubscripts.size(); ++i) {
982-
const SCEV *GEPS = GEPSubscripts[i];
983-
const SCEV *AIS = ArrayInfoSubscripts[i];
984-
// FIXME: there's no good way to compare two scevs: don't abort, warn.
985-
if (GEPS != AIS || !SE->getMinusSCEV(GEPS, AIS)->isZero()) {
986-
LLVM_DEBUG({
987-
dbgs() << "WARN: Subscript arrays differ at index " << i << "!\n";
988-
dbgs() << " GEP subscript[" << i << "]: " << *GEPSubscripts[i]
975+
for (size_t I : seq<size_t>(GEPSubscripts.size())) {
976+
const SCEV *GEPS = GEPSubscripts[I];
977+
const SCEV *AIS = ArrayInfoSubscripts[I];
978+
// FIXME: there's no good way to compare two scevs: don't abort, warn.
979+
if (GEPS != AIS || !SE->getMinusSCEV(GEPS, AIS)->isZero()) {
980+
dbgs() << "WARN: Subscript arrays differ at index " << I << "!\n";
981+
dbgs() << " GEP subscript[" << I << "]: " << *GEPSubscripts[I]
989982
<< "\n"
990-
<< " ArrayInfo subscript[" << i
991-
<< "]: " << *ArrayInfoSubscripts[i] << "\n";
992-
});
983+
<< " ArrayInfo subscript[" << I
984+
<< "]: " << *ArrayInfoSubscripts[I] << "\n";
985+
}
993986
}
994-
}
995987

996-
LLVM_DEBUG(dbgs() << "SUCCESS: Both delinearization methods produced "
997-
"identical results\n");
998-
} else if (GEPSuccess && !ArrayInfoSuccess) {
999-
LLVM_DEBUG({
988+
dbgs() << "SUCCESS: Both delinearization methods produced "
989+
"identical results\n";
990+
} else if (GEPSuccess && !ArrayInfoSuccess) {
1000991
dbgs() << "WARNING: array_info failed and GEP analysis succeeded.\n";
1001992
dbgs() << " Instruction: " << *Inst << "\n";
1002993
dbgs() << " Using GEP analysis results despite array_info failure\n";
1003-
});
1004-
} else if (!GEPSuccess && ArrayInfoSuccess) {
1005-
LLVM_DEBUG({
994+
} else if (!GEPSuccess && ArrayInfoSuccess) {
1006995
dbgs() << "WARNING: GEP failed and array_info analysis succeeded.\n";
1007996
dbgs() << " Instruction: " << *Inst << "\n";
1008997
dbgs() << " Using array_info analysis results despite GEP failure\n";
1009-
});
1010-
} else if (!GEPSuccess && !ArrayInfoSuccess) {
1011-
LLVM_DEBUG({
998+
} else if (!GEPSuccess && !ArrayInfoSuccess) {
1012999
dbgs() << "WARNING: both GEP and array_info analysis failed.\n";
10131000
dbgs() << " Instruction: " << *Inst << "\n";
1014-
});
1015-
}
1001+
}
1002+
});
10161003

10171004
// Choose which result to use.
10181005
// Prefer array_info when available.
@@ -1081,7 +1068,7 @@ void printDelinearization(raw_ostream &O, Function *F, LoopInfo *LI,
10811068

10821069
SmallVector<const SCEV *, 3> Subscripts, Sizes;
10831070
auto IsDelinearizationFailed = [&]() {
1084-
return Subscripts.size() == 0 || Sizes.size() == 0;
1071+
return Subscripts.empty() || Sizes.empty();
10851072
};
10861073

10871074
const SCEV *ElementSize = SE->getElementSize(&Inst);
@@ -1117,8 +1104,8 @@ void printDelinearization(raw_ostream &O, Function *F, LoopInfo *LI,
11171104
// Handle different size relationships between Subscripts and Sizes.
11181105
if (NumSizes > 0) {
11191106
// Print array dimensions (all but the last, which is element size).
1120-
for (int i = 0; i < NumSizes - 1; i++)
1121-
O << "[" << *Sizes[i] << "]";
1107+
for (const SCEV *Size : ArrayRef(Sizes).drop_back())
1108+
O << "[" << *Size << "]";
11221109

11231110
// Print element size (last element in Sizes array).
11241111
O << " with elements of " << *Sizes[NumSizes - 1] << " bytes.\n";

0 commit comments

Comments
 (0)