Skip to content

Commit

Permalink
[ValueTracking] Add CharWidth argument to getConstantStringInfo (NFC)
Browse files Browse the repository at this point in the history
The method assumes that host chars and target chars have the same width.
Add a CharWidth argument so that it can bail out if the requested char
width differs from the host char width.

Alternatively, the check could be done at call sites, but this is more
error-prone.

In the future, this method will be replaced with a different one that
allows host/target chars to have different widths. The prototype will
be the same except that StringRef is replaced with something that is
byte width agnostic. Adding CharWidth argument now reduces the future
diff.
  • Loading branch information
s-barannikov committed Oct 27, 2024
1 parent 61be980 commit d27bc30
Show file tree
Hide file tree
Showing 10 changed files with 96 additions and 46 deletions.
4 changes: 2 additions & 2 deletions clang/lib/CodeGen/CGBuiltin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18585,7 +18585,7 @@ void CodeGenFunction::ProcessOrderScopeAMDGCN(Value *Order, Value *Scope,

// Some of the atomic builtins take the scope as a string name.
StringRef scp;
if (llvm::getConstantStringInfo(Scope, scp)) {
if (llvm::getConstantStringInfo(Scope, scp, /*CharWidth=*/8)) {
SSID = getLLVMContext().getOrInsertSyncScopeID(scp);
return;
}
Expand Down Expand Up @@ -18972,7 +18972,7 @@ void CodeGenFunction::AddAMDGPUFenceAddressSpaceMMRA(llvm::Instruction *Inst,
for (unsigned K = 2; K < E->getNumArgs(); ++K) {
llvm::Value *V = EmitScalarExpr(E->getArg(K));
StringRef AS;
if (llvm::getConstantStringInfo(V, AS)) {
if (llvm::getConstantStringInfo(V, AS, /*CharWidth=*/8)) {
MMRAs.push_back({Tag, AS});
// TODO: Delete the resulting unused constant?
continue;
Expand Down
2 changes: 1 addition & 1 deletion llvm/include/llvm/Analysis/ValueTracking.h
Original file line number Diff line number Diff line change
Expand Up @@ -695,7 +695,7 @@ bool getConstantDataArrayInfo(const Value *V, ConstantDataArraySlice &Slice,
/// character by default. If TrimAtNul is set to false, then this returns any
/// trailing null characters as well as any other characters that come after
/// it.
bool getConstantStringInfo(const Value *V, StringRef &Str,
bool getConstantStringInfo(const Value *V, StringRef &Str, unsigned CharWidth,
bool TrimAtNul = true);

/// If we can compute the length of the string pointed to by the specified
Expand Down
7 changes: 5 additions & 2 deletions llvm/lib/Analysis/ValueTracking.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6492,9 +6492,12 @@ bool llvm::getConstantDataArrayInfo(const Value *V,
/// return true. When TrimAtNul is set, Str will contain only the bytes up
/// to but not including the first nul. Return false on failure.
bool llvm::getConstantStringInfo(const Value *V, StringRef &Str,
bool TrimAtNul) {
unsigned CharWidth, bool TrimAtNul) {
if (CharWidth != CHAR_BIT)
return false;

ConstantDataArraySlice Slice;
if (!getConstantDataArrayInfo(V, Slice, 8))
if (!getConstantDataArrayInfo(V, Slice, CharWidth))
return false;

if (Slice.Array == nullptr) {
Expand Down
4 changes: 2 additions & 2 deletions llvm/lib/Target/AMDGPU/AMDGPUPrintfRuntimeBinding.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ static_assert(NonLiteralStr.size() == 3);

static StringRef getAsConstantStr(Value *V) {
StringRef S;
if (!getConstantStringInfo(V, S))
if (!getConstantStringInfo(V, S, /*CharWidth=*/8))
S = NonLiteralStr;

return S;
Expand Down Expand Up @@ -161,7 +161,7 @@ bool AMDGPUPrintfRuntimeBindingImpl::lowerPrintfForGpu(Module &M) {
Value *Op = CI->getArgOperand(0);

StringRef FormatStr;
if (!getConstantStringInfo(Op, FormatStr)) {
if (!getConstantStringInfo(Op, FormatStr, /*CharWidth=*/8)) {
Value *Stripped = Op->stripPointerCasts();
if (!isa<UndefValue>(Stripped) && !isa<ConstantPointerNull>(Stripped))
diagnoseInvalidFormatString(CI);
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Target/SPIRV/SPIRVAsmPrinter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -561,7 +561,7 @@ void SPIRVAsmPrinter::outputAnnotations(const Module &M) {
cast<GlobalVariable>(CS->getOperand(1)->stripPointerCasts());

StringRef AnnotationString;
getConstantStringInfo(GV, AnnotationString);
getConstantStringInfo(GV, AnnotationString, /*CharWidth=*/8);
MCInst Inst;
Inst.setOpcode(SPIRV::OpDecorate);
Inst.addOperand(MCOperand::createReg(Reg));
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Target/SPIRV/SPIRVPrepareFunctions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ static std::string getAnnotation(Value *AnnoVal, Value *OptAnnoVal) {
std::string Anno;
if (auto *C = dyn_cast_or_null<Constant>(AnnoVal)) {
StringRef Str;
if (getConstantStringInfo(C, Str))
if (getConstantStringInfo(C, Str, /*CharWidth=*/8))
Anno = Str;
}
// handle optional annotation parameter in a way that Khronos Translator do
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -579,7 +579,7 @@ void WebAssemblyAsmPrinter::EmitFunctionAttributes(Module &M) {
// The second field is a pointer to a global annotation string.
auto *GV = cast<GlobalVariable>(CS->getOperand(1)->stripPointerCasts());
StringRef AnnotationString;
getConstantStringInfo(GV, AnnotationString);
getConstantStringInfo(GV, AnnotationString, /*CharWidth=*/8);
auto *Sym = cast<MCSymbolWasm>(getSymbol(F));
CustomSections[AnnotationString].push_back(Sym);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -966,6 +966,8 @@ class StrNCmpInliner {
/// handled by the instcombine pass.
///
bool StrNCmpInliner::optimizeStrNCmp() {
unsigned CharWidth = DL.getByteWidth();

if (StrNCmpInlineThreshold < 2)
return false;

Expand All @@ -979,8 +981,10 @@ bool StrNCmpInliner::optimizeStrNCmp() {
return false;

StringRef Str1, Str2;
bool HasStr1 = getConstantStringInfo(Str1P, Str1, /*TrimAtNul=*/false);
bool HasStr2 = getConstantStringInfo(Str2P, Str2, /*TrimAtNul=*/false);
bool HasStr1 =
getConstantStringInfo(Str1P, Str1, CharWidth, /*TrimAtNul=*/false);
bool HasStr2 =
getConstantStringInfo(Str2P, Str2, CharWidth, /*TrimAtNul=*/false);
if (HasStr1 == HasStr2)
return false;

Expand Down Expand Up @@ -1110,12 +1114,14 @@ void StrNCmpInliner::inlineCompare(Value *LHS, StringRef RHS, uint64_t N,
/// Convert memchr with a small constant string into a switch
static bool foldMemChr(CallInst *Call, DomTreeUpdater *DTU,
const DataLayout &DL) {
unsigned CharWidth = DL.getByteWidth();

if (isa<Constant>(Call->getArgOperand(1)))
return false;

StringRef Str;
Value *Base = Call->getArgOperand(0);
if (!getConstantStringInfo(Base, Str, /*TrimAtNul=*/false))
if (!getConstantStringInfo(Base, Str, CharWidth, /*TrimAtNul=*/false))
return false;

uint64_t N = Str.size();
Expand Down
4 changes: 2 additions & 2 deletions llvm/lib/Transforms/Utils/AMDGPUEmitPrintf.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ static Value *callBufferedPrintfStart(
for (size_t i = 1; i < Args.size(); i++) {
if (SpecIsCString.test(i)) {
StringRef ArgStr;
if (getConstantStringInfo(Args[i], ArgStr)) {
if (getConstantStringInfo(Args[i], ArgStr, /*CharWidth=*/8)) {
auto alignedLen = alignTo(ArgStr.size() + 1, 8);
StringContents.push_back(StringData(
ArgStr,
Expand Down Expand Up @@ -432,7 +432,7 @@ Value *llvm::emitAMDGPUPrintfCall(IRBuilder<> &Builder, ArrayRef<Value *> Args,
SparseBitVector<8> SpecIsCString;
StringRef FmtStr;

if (getConstantStringInfo(Fmt, FmtStr))
if (getConstantStringInfo(Fmt, FmtStr, /*CharWidth=*/8))
locateCStrings(SpecIsCString, FmtStr);

if (IsBuffered) {
Expand Down
Loading

0 comments on commit d27bc30

Please sign in to comment.