Skip to content

Commit 183acdd

Browse files
authored
[GlobalOpt] Revert global widening transform (#144652)
Partially reverts e37d736. The transform has a number of correctness and code quality issues, and will benefit from a from-scratch re-review more than incremental fixes. The correctness issues are hinted at in #144641, but I think it needs a larger rework to stop working on ArrayTypes and the implementation could use some other improvements (like callInstIsMemcpy should just be `dyn_cast<MemCpyInst>`). I can comment in more detail on a resubmission of the patch.
1 parent f01a793 commit 183acdd

14 files changed

+0
-534
lines changed

llvm/lib/Transforms/IPO/GlobalOpt.cpp

Lines changed: 0 additions & 165 deletions
Original file line numberDiff line numberDiff line change
@@ -92,8 +92,6 @@ STATISTIC(NumInternalFunc, "Number of internal functions");
9292
STATISTIC(NumColdCC, "Number of functions marked coldcc");
9393
STATISTIC(NumIFuncsResolved, "Number of statically resolved IFuncs");
9494
STATISTIC(NumIFuncsDeleted, "Number of IFuncs removed");
95-
STATISTIC(NumGlobalArraysPadded,
96-
"Number of global arrays padded to alignment boundary");
9795

9896
static cl::opt<bool>
9997
OptimizeNonFMVCallers("optimize-non-fmv-callers",
@@ -2037,165 +2035,6 @@ OptimizeFunctions(Module &M,
20372035
return Changed;
20382036
}
20392037

2040-
static bool callInstIsMemcpy(CallInst *CI) {
2041-
if (!CI)
2042-
return false;
2043-
2044-
Function *F = CI->getCalledFunction();
2045-
if (!F || !F->isIntrinsic() || F->getIntrinsicID() != Intrinsic::memcpy)
2046-
return false;
2047-
2048-
return true;
2049-
}
2050-
2051-
static bool destArrayCanBeWidened(CallInst *CI) {
2052-
auto *IsVolatile = dyn_cast<ConstantInt>(CI->getArgOperand(3));
2053-
auto *Alloca = dyn_cast<AllocaInst>(CI->getArgOperand(0));
2054-
2055-
if (!Alloca || !IsVolatile || IsVolatile->isOne())
2056-
return false;
2057-
2058-
if (!Alloca->isStaticAlloca())
2059-
return false;
2060-
2061-
if (!Alloca->getAllocatedType()->isArrayTy())
2062-
return false;
2063-
2064-
return true;
2065-
}
2066-
2067-
static GlobalVariable *widenGlobalVariable(GlobalVariable *OldVar,
2068-
unsigned NumBytesToPad,
2069-
unsigned NumBytesToCopy) {
2070-
if (!OldVar->hasInitializer())
2071-
return nullptr;
2072-
2073-
ConstantDataArray *DataArray =
2074-
dyn_cast<ConstantDataArray>(OldVar->getInitializer());
2075-
if (!DataArray)
2076-
return nullptr;
2077-
2078-
// Update to be word aligned (memcpy(...,X,...))
2079-
// create replacement with padded null bytes.
2080-
StringRef Data = DataArray->getRawDataValues();
2081-
std::vector<uint8_t> StrData(Data.begin(), Data.end());
2082-
for (unsigned int p = 0; p < NumBytesToPad; p++)
2083-
StrData.push_back('\0');
2084-
auto Arr = ArrayRef(StrData.data(), NumBytesToCopy + NumBytesToPad);
2085-
// Create new padded version of global variable.
2086-
Constant *SourceReplace = ConstantDataArray::get(OldVar->getContext(), Arr);
2087-
GlobalVariable *NewGV = new GlobalVariable(
2088-
*(OldVar->getParent()), SourceReplace->getType(), true,
2089-
OldVar->getLinkage(), SourceReplace, SourceReplace->getName());
2090-
// Copy any other attributes from original global variable
2091-
// e.g. unamed_addr
2092-
NewGV->copyAttributesFrom(OldVar);
2093-
NewGV->takeName(OldVar);
2094-
return NewGV;
2095-
}
2096-
2097-
static void widenDestArray(CallInst *CI, const unsigned NumBytesToPad,
2098-
const unsigned NumBytesToCopy,
2099-
ConstantDataArray *SourceDataArray) {
2100-
2101-
auto *Alloca = dyn_cast<AllocaInst>(CI->getArgOperand(0));
2102-
if (Alloca) {
2103-
unsigned ElementByteWidth = SourceDataArray->getElementByteSize();
2104-
unsigned int TotalBytes = NumBytesToCopy + NumBytesToPad;
2105-
unsigned NumElementsToCopy = divideCeil(TotalBytes, ElementByteWidth);
2106-
// Update destination array to be word aligned (memcpy(X,...,...))
2107-
IRBuilder<> BuildAlloca(Alloca);
2108-
AllocaInst *NewAlloca = BuildAlloca.CreateAlloca(ArrayType::get(
2109-
Alloca->getAllocatedType()->getArrayElementType(), NumElementsToCopy));
2110-
NewAlloca->takeName(Alloca);
2111-
NewAlloca->setAlignment(Alloca->getAlign());
2112-
Alloca->replaceAllUsesWith(NewAlloca);
2113-
Alloca->eraseFromParent();
2114-
}
2115-
}
2116-
2117-
static bool tryWidenGlobalArrayAndDests(GlobalVariable *SourceVar,
2118-
const unsigned NumBytesToPad,
2119-
const unsigned NumBytesToCopy,
2120-
ConstantInt *BytesToCopyOp,
2121-
ConstantDataArray *SourceDataArray) {
2122-
auto *NewSourceGV =
2123-
widenGlobalVariable(SourceVar, NumBytesToPad, NumBytesToCopy);
2124-
if (!NewSourceGV)
2125-
return false;
2126-
2127-
// Update arguments of remaining uses that
2128-
// are memcpys.
2129-
for (auto *User : SourceVar->users()) {
2130-
auto *CI = dyn_cast<CallInst>(User);
2131-
if (!callInstIsMemcpy(CI) || !destArrayCanBeWidened(CI))
2132-
continue;
2133-
2134-
if (CI->getArgOperand(1) != SourceVar)
2135-
continue;
2136-
2137-
widenDestArray(CI, NumBytesToPad, NumBytesToCopy, SourceDataArray);
2138-
2139-
CI->setArgOperand(2, ConstantInt::get(BytesToCopyOp->getType(),
2140-
NumBytesToCopy + NumBytesToPad));
2141-
}
2142-
SourceVar->replaceAllUsesWith(NewSourceGV);
2143-
2144-
NumGlobalArraysPadded++;
2145-
return true;
2146-
}
2147-
2148-
static bool tryWidenGlobalArraysUsedByMemcpy(
2149-
GlobalVariable *GV,
2150-
function_ref<TargetTransformInfo &(Function &)> GetTTI) {
2151-
2152-
if (!GV->hasInitializer() || !GV->isConstant() || !GV->hasLocalLinkage() ||
2153-
!GV->hasGlobalUnnamedAddr())
2154-
return false;
2155-
2156-
for (auto *User : GV->users()) {
2157-
CallInst *CI = dyn_cast<CallInst>(User);
2158-
if (!callInstIsMemcpy(CI) || !destArrayCanBeWidened(CI))
2159-
continue;
2160-
2161-
auto *BytesToCopyOp = dyn_cast<ConstantInt>(CI->getArgOperand(2));
2162-
if (!BytesToCopyOp)
2163-
continue;
2164-
2165-
ConstantDataArray *SourceDataArray =
2166-
dyn_cast<ConstantDataArray>(GV->getInitializer());
2167-
if (!SourceDataArray)
2168-
continue;
2169-
2170-
unsigned NumBytesToCopy = BytesToCopyOp->getZExtValue();
2171-
2172-
auto *Alloca = cast<AllocaInst>(CI->getArgOperand(0));
2173-
uint64_t DZSize = Alloca->getAllocatedType()->getArrayNumElements();
2174-
uint64_t SZSize = SourceDataArray->getType()->getNumElements();
2175-
unsigned ElementByteWidth = SourceDataArray->getElementByteSize();
2176-
// Calculate the number of elements to copy while avoiding floored
2177-
// division of integers returning wrong values i.e. copying one byte
2178-
// from an array of i16 would yield 0 elements to copy as supposed to 1.
2179-
unsigned NumElementsToCopy = divideCeil(NumBytesToCopy, ElementByteWidth);
2180-
2181-
// For safety purposes lets add a constraint and only pad when
2182-
// NumElementsToCopy == destination array size ==
2183-
// source which is a constant
2184-
if (NumElementsToCopy != DZSize || DZSize != SZSize)
2185-
continue;
2186-
2187-
unsigned NumBytesToPad =
2188-
GetTTI(*CI->getFunction())
2189-
.getNumBytesToPadGlobalArray(NumBytesToCopy,
2190-
SourceDataArray->getType());
2191-
if (NumBytesToPad) {
2192-
return tryWidenGlobalArrayAndDests(GV, NumBytesToPad, NumBytesToCopy,
2193-
BytesToCopyOp, SourceDataArray);
2194-
}
2195-
}
2196-
return false;
2197-
}
2198-
21992038
static bool
22002039
OptimizeGlobalVars(Module &M,
22012040
function_ref<TargetTransformInfo &(Function &)> GetTTI,
@@ -2225,10 +2064,6 @@ OptimizeGlobalVars(Module &M,
22252064
continue;
22262065
}
22272066

2228-
// For global variable arrays called in a memcpy
2229-
// we try to pad to nearest valid alignment boundary
2230-
Changed |= tryWidenGlobalArraysUsedByMemcpy(&GV, GetTTI);
2231-
22322067
Changed |= processGlobal(GV, GetTTI, GetTLI, LookupDomTree);
22332068
}
22342069
return Changed;

llvm/test/Transforms/GlobalOpt/ARM/arm-widen-dest-non-array.ll

Lines changed: 0 additions & 39 deletions
This file was deleted.

llvm/test/Transforms/GlobalOpt/ARM/arm-widen-global-dest.ll

Lines changed: 0 additions & 28 deletions
This file was deleted.

llvm/test/Transforms/GlobalOpt/ARM/arm-widen-non-byte-array.ll

Lines changed: 0 additions & 22 deletions
This file was deleted.

llvm/test/Transforms/GlobalOpt/ARM/arm-widen-non-const-global.ll

Lines changed: 0 additions & 21 deletions
This file was deleted.

llvm/test/Transforms/GlobalOpt/ARM/arm-widen-string-multi-use.ll

Lines changed: 0 additions & 33 deletions
This file was deleted.

llvm/test/Transforms/GlobalOpt/ARM/arm-widen-strings-1.ll

Lines changed: 0 additions & 21 deletions
This file was deleted.

llvm/test/Transforms/GlobalOpt/ARM/arm-widen-strings-2.ll

Lines changed: 0 additions & 21 deletions
This file was deleted.

0 commit comments

Comments
 (0)