Skip to content

Commit d3b994b

Browse files
committed
[DAGCombiner] Add option to force isNarrowingProfitable in tests. NFC
This patch only adds a way to override the TLI.isNarrowingProfitable check in DAGCombiner::ReduceLoadOpStoreWidth by using the option -combiner-reduce-load-op-store-width-force-narrowing-profitable. Idea is that it should be simpler to for example add lit tests verifying that the code is correct for big-endian (which otherwise is difficult since there are no in-tree big-endian targets that is overriding TLI.isNarrowingProfitable).
1 parent c68a06b commit d3b994b

File tree

1 file changed

+22
-3
lines changed

1 file changed

+22
-3
lines changed

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,11 @@ static cl::opt<bool> EnableReduceLoadOpStoreWidth(
138138
"combiner-reduce-load-op-store-width", cl::Hidden, cl::init(true),
139139
cl::desc("DAG combiner enable reducing the width of load/op/store "
140140
"sequence"));
141+
static cl::opt<bool> ReduceLoadOpStoreWidthForceNarrowingProfitable(
142+
"combiner-reduce-load-op-store-width-force-narrowing-profitable",
143+
cl::Hidden, cl::init(false),
144+
cl::desc("DAG combiner force override the narrowing profitable check when"
145+
"reducing the width of load/op/store sequences"));
141146

142147
static cl::opt<bool> EnableShrinkLoadReplaceStoreWithStore(
143148
"combiner-shrink-load-replace-store-with-store", cl::Hidden, cl::init(true),
@@ -20348,15 +20353,29 @@ SDValue DAGCombiner::ReduceLoadOpStoreWidth(SDNode *N) {
2034820353
EVT NewVT = EVT::getIntegerVT(*DAG.getContext(), NewBW);
2034920354
// The narrowing should be profitable, the load/store operation should be
2035020355
// legal (or custom) and the store size should be equal to the NewVT width.
20351-
while (NewBW < BitWidth && (NewVT.getStoreSizeInBits() != NewBW ||
20352-
!TLI.isOperationLegalOrCustom(Opc, NewVT) ||
20353-
!TLI.isNarrowingProfitable(N, VT, NewVT))) {
20356+
while (NewBW < BitWidth &&
20357+
(NewVT.getStoreSizeInBits() != NewBW ||
20358+
!TLI.isOperationLegalOrCustom(Opc, NewVT) ||
20359+
!(TLI.isNarrowingProfitable(N, VT, NewVT) ||
20360+
ReduceLoadOpStoreWidthForceNarrowingProfitable))) {
2035420361
NewBW = NextPowerOf2(NewBW);
2035520362
NewVT = EVT::getIntegerVT(*DAG.getContext(), NewBW);
2035620363
}
2035720364
if (NewBW >= BitWidth)
2035820365
return SDValue();
2035920366

20367+
// TODO: For big-endian we probably want to align given the most significant
20368+
// bit being modified instead of adjusting ShAmt based on least significant
20369+
// bits. This to reduce the risk of failing on the alignment check below. If
20370+
// for example VT.getStoreSize()==5 and Imm is 0x0000ffff00, then we want to
20371+
// find NewBW=16, and we want to load/store with a PtrOff set to 2. But then
20372+
// ShAmt should be set to 8, which isn't a multiple of NewBW. But given
20373+
// that isNarrowingProfitable doesn't seem to be overridden for any in-tree
20374+
// big-endian target, then the support for big-endian here isn't covered by
20375+
// any in-tree lit tests, so it is unfortunately not highly optimized
20376+
// either. It should be possible to improve that by using
20377+
// ReduceLoadOpStoreWidthForceNarrowingProfitable.
20378+
2036020379
// If the lsb that is modified does not start at the type bitwidth boundary,
2036120380
// align to start at the previous boundary.
2036220381
ShAmt = ShAmt - (ShAmt % NewBW);

0 commit comments

Comments
 (0)