Normalize interaction with boolean attributes
Such attributes can either be unset, or set to "true" or "false" (as string).
throughout the codebase, this led to inelegant checks ranging from
if (Fn->getFnAttribute("no-jump-tables").getValueAsString() == "true")
to
if (Fn->hasAttribute("no-jump-tables") && Fn->getFnAttribute("no-jump-tables").getValueAsString() == "true")
Introduce a getValueAsBool that normalize the check, with the following
behavior:
no attributes or attribute set to "false" => return false
attribute set to "true" => return true
Differential Revision: https://reviews.llvm.org/D99299
GitOrigin-RevId: d6de1e1a71406c75a4ea4d5a2fe84289f07ea3a1
diff --git a/include/llvm/CodeGen/TargetLowering.h b/include/llvm/CodeGen/TargetLowering.h
index 4b964dc..7c77787 100644
--- a/include/llvm/CodeGen/TargetLowering.h
+++ b/include/llvm/CodeGen/TargetLowering.h
@@ -1144,7 +1144,7 @@
/// Return true if lowering to a jump table is allowed.
virtual bool areJTsAllowed(const Function *Fn) const {
- if (Fn->getFnAttribute("no-jump-tables").getValueAsString() == "true")
+ if (Fn->getFnAttribute("no-jump-tables").getValueAsBool())
return false;
return isOperationLegalOrCustom(ISD::BR_JT, MVT::Other) ||
diff --git a/include/llvm/IR/Attributes.h b/include/llvm/IR/Attributes.h
index 8a87a56..50047e2 100644
--- a/include/llvm/IR/Attributes.h
+++ b/include/llvm/IR/Attributes.h
@@ -168,6 +168,10 @@
/// attribute be an integer attribute.
uint64_t getValueAsInt() const;
+ /// Return the attribute's value as a boolean. This requires that the
+ /// attribute be a string attribute.
+ bool getValueAsBool() const;
+
/// Return the attribute's kind as a string. This requires the
/// attribute to be a string attribute.
StringRef getKindAsString() const;
diff --git a/lib/Analysis/IVDescriptors.cpp b/lib/Analysis/IVDescriptors.cpp
index 3427150..945d0d3 100644
--- a/lib/Analysis/IVDescriptors.cpp
+++ b/lib/Analysis/IVDescriptors.cpp
@@ -653,9 +653,9 @@
Function &F = *Header->getParent();
FastMathFlags FMF;
FMF.setNoNaNs(
- F.getFnAttribute("no-nans-fp-math").getValueAsString() == "true");
+ F.getFnAttribute("no-nans-fp-math").getValueAsBool());
FMF.setNoSignedZeros(
- F.getFnAttribute("no-signed-zeros-fp-math").getValueAsString() == "true");
+ F.getFnAttribute("no-signed-zeros-fp-math").getValueAsBool());
if (AddReductionVar(Phi, RecurKind::Add, TheLoop, FMF, RedDes, DB, AC, DT)) {
LLVM_DEBUG(dbgs() << "Found an ADD reduction PHI." << *Phi << "\n");
diff --git a/lib/CodeGen/SelectionDAG/FastISel.cpp b/lib/CodeGen/SelectionDAG/FastISel.cpp
index 79ac13c..faf3a84 100644
--- a/lib/CodeGen/SelectionDAG/FastISel.cpp
+++ b/lib/CodeGen/SelectionDAG/FastISel.cpp
@@ -1145,7 +1145,7 @@
IsTailCall = false;
if (IsTailCall && MF->getFunction()
.getFnAttribute("disable-tail-calls")
- .getValueAsString() == "true")
+ .getValueAsBool())
IsTailCall = false;
CallLoweringInfo CLI;
diff --git a/lib/CodeGen/SelectionDAG/TargetLowering.cpp b/lib/CodeGen/SelectionDAG/TargetLowering.cpp
index fcb8d9b..870c4bf 100644
--- a/lib/CodeGen/SelectionDAG/TargetLowering.cpp
+++ b/lib/CodeGen/SelectionDAG/TargetLowering.cpp
@@ -53,7 +53,7 @@
const Function &F = DAG.getMachineFunction().getFunction();
// First, check if tail calls have been disabled in this function.
- if (F.getFnAttribute("disable-tail-calls").getValueAsString() == "true")
+ if (F.getFnAttribute("disable-tail-calls").getValueAsBool())
return false;
// Conservatively require the attributes of the call to match those of
diff --git a/lib/IR/AttributeImpl.h b/lib/IR/AttributeImpl.h
index 60e2ec2..3b297a9 100644
--- a/lib/IR/AttributeImpl.h
+++ b/lib/IR/AttributeImpl.h
@@ -64,6 +64,7 @@
Attribute::AttrKind getKindAsEnum() const;
uint64_t getValueAsInt() const;
+ bool getValueAsBool() const;
StringRef getKindAsString() const;
StringRef getValueAsString() const;
diff --git a/lib/IR/Attributes.cpp b/lib/IR/Attributes.cpp
index 60ad3b8..30730a4 100644
--- a/lib/IR/Attributes.cpp
+++ b/lib/IR/Attributes.cpp
@@ -287,6 +287,13 @@
return pImpl->getValueAsInt();
}
+bool Attribute::getValueAsBool() const {
+ if (!pImpl) return false;
+ assert(isStringAttribute() &&
+ "Expected the attribute to be a string attribute!");
+ return pImpl->getValueAsBool();
+}
+
StringRef Attribute::getKindAsString() const {
if (!pImpl) return {};
assert(isStringAttribute() &&
@@ -650,6 +657,11 @@
return static_cast<const IntAttributeImpl *>(this)->getValue();
}
+bool AttributeImpl::getValueAsBool() const {
+ assert(getValueAsString().empty() || getValueAsString() == "false" || getValueAsString() == "true");
+ return getValueAsString() == "true";
+}
+
StringRef AttributeImpl::getKindAsString() const {
assert(isStringAttribute());
return static_cast<const StringAttributeImpl *>(this)->getStringKind();
diff --git a/lib/IR/Verifier.cpp b/lib/IR/Verifier.cpp
index d0bfa7e..9d3c791 100644
--- a/lib/IR/Verifier.cpp
+++ b/lib/IR/Verifier.cpp
@@ -1717,8 +1717,21 @@
void Verifier::verifyAttributeTypes(AttributeSet Attrs, bool IsFunction,
const Value *V) {
for (Attribute A : Attrs) {
- if (A.isStringAttribute())
+
+ if (A.isStringAttribute()) {
+#define GET_ATTR_NAMES
+#define ATTRIBUTE_ENUM(ENUM_NAME, DISPLAY_NAME)
+#define ATTRIBUTE_STRBOOL(ENUM_NAME, DISPLAY_NAME) \
+ if (A.getKindAsString() == #DISPLAY_NAME) { \
+ auto V = A.getValueAsString(); \
+ if (!(V.empty() || V == "true" || V == "false")) \
+ CheckFailed("invalid value for '" #DISPLAY_NAME "' attribute: " + V + \
+ ""); \
+ }
+
+#include "llvm/IR/Attributes.inc"
continue;
+ }
if (A.isIntAttribute() !=
Attribute::doesAttrKindHaveArgument(A.getKindAsEnum())) {
diff --git a/lib/Target/AMDGPU/AMDGPUCodeGenPrepare.cpp b/lib/Target/AMDGPU/AMDGPUCodeGenPrepare.cpp
index 2556996..154d9c3 100644
--- a/lib/Target/AMDGPU/AMDGPUCodeGenPrepare.cpp
+++ b/lib/Target/AMDGPU/AMDGPUCodeGenPrepare.cpp
@@ -809,7 +809,7 @@
static bool hasUnsafeFPMath(const Function &F) {
Attribute Attr = F.getFnAttribute("unsafe-fp-math");
- return Attr.getValueAsString() == "true";
+ return Attr.getValueAsBool();
}
static std::pair<Value*, Value*> getMul64(IRBuilder<> &Builder,
diff --git a/lib/Target/AMDGPU/AMDGPULibCalls.cpp b/lib/Target/AMDGPU/AMDGPULibCalls.cpp
index 6b7f572..2b2242e 100644
--- a/lib/Target/AMDGPU/AMDGPULibCalls.cpp
+++ b/lib/Target/AMDGPU/AMDGPULibCalls.cpp
@@ -476,7 +476,7 @@
return true;
const Function *F = CI->getParent()->getParent();
Attribute Attr = F->getFnAttribute("unsafe-fp-math");
- return Attr.getValueAsString() == "true";
+ return Attr.getValueAsBool();
}
bool AMDGPULibCalls::useNativeFunc(const StringRef F) const {
diff --git a/lib/Target/AMDGPU/AMDGPULowerKernelAttributes.cpp b/lib/Target/AMDGPU/AMDGPULowerKernelAttributes.cpp
index 9ab6a52..17b75e0 100644
--- a/lib/Target/AMDGPU/AMDGPULowerKernelAttributes.cpp
+++ b/lib/Target/AMDGPU/AMDGPULowerKernelAttributes.cpp
@@ -67,7 +67,7 @@
const bool HasReqdWorkGroupSize = MD && MD->getNumOperands() == 3;
const bool HasUniformWorkGroupSize =
- F->getFnAttribute("uniform-work-group-size").getValueAsString() == "true";
+ F->getFnAttribute("uniform-work-group-size").getValueAsBool();
if (!HasReqdWorkGroupSize && !HasUniformWorkGroupSize)
return false;
diff --git a/lib/Target/AMDGPU/AMDGPUMachineFunction.cpp b/lib/Target/AMDGPU/AMDGPUMachineFunction.cpp
index 5134497..07806dd 100644
--- a/lib/Target/AMDGPU/AMDGPUMachineFunction.cpp
+++ b/lib/Target/AMDGPU/AMDGPUMachineFunction.cpp
@@ -28,12 +28,10 @@
const Function &F = MF.getFunction();
Attribute MemBoundAttr = F.getFnAttribute("amdgpu-memory-bound");
- MemoryBound = MemBoundAttr.isStringAttribute() &&
- MemBoundAttr.getValueAsString() == "true";
+ MemoryBound = MemBoundAttr.getValueAsBool();
Attribute WaveLimitAttr = F.getFnAttribute("amdgpu-wave-limiter");
- WaveLimiter = WaveLimitAttr.isStringAttribute() &&
- WaveLimitAttr.getValueAsString() == "true";
+ WaveLimiter = WaveLimitAttr.getValueAsBool();
CallingConv::ID CC = F.getCallingConv();
if (CC == CallingConv::AMDGPU_KERNEL || CC == CallingConv::SPIR_KERNEL)
diff --git a/lib/Target/ARM/ARMTargetMachine.cpp b/lib/Target/ARM/ARMTargetMachine.cpp
index c09df07..033b98a 100644
--- a/lib/Target/ARM/ARMTargetMachine.cpp
+++ b/lib/Target/ARM/ARMTargetMachine.cpp
@@ -274,8 +274,7 @@
// function before we can generate a subtarget. We also need to use
// it as a key for the subtarget since that can be the only difference
// between two functions.
- bool SoftFloat =
- F.getFnAttribute("use-soft-float").getValueAsString() == "true";
+ bool SoftFloat = F.getFnAttribute("use-soft-float").getValueAsBool();
// If the soft float attribute is set on the function turn on the soft float
// subtarget feature.
if (SoftFloat)
diff --git a/lib/Target/Hexagon/HexagonTargetMachine.cpp b/lib/Target/Hexagon/HexagonTargetMachine.cpp
index 9195bb3..9c0bcd9 100644
--- a/lib/Target/Hexagon/HexagonTargetMachine.cpp
+++ b/lib/Target/Hexagon/HexagonTargetMachine.cpp
@@ -251,8 +251,7 @@
// Creating a separate target feature is not strictly necessary, it only
// exists to make "unsafe-fp-math" force creating a new subtarget.
- if (FnAttrs.hasFnAttribute("unsafe-fp-math") &&
- F.getFnAttribute("unsafe-fp-math").getValueAsString() == "true")
+ if (F.getFnAttribute("unsafe-fp-math").getValueAsBool())
FS = FS.empty() ? "+unsafe-fp" : "+unsafe-fp," + FS;
auto &I = SubtargetMap[CPU + FS];
diff --git a/lib/Target/M68k/M68kISelLowering.cpp b/lib/Target/M68k/M68kISelLowering.cpp
index 8402bbb..7c2e253 100644
--- a/lib/Target/M68k/M68kISelLowering.cpp
+++ b/lib/Target/M68k/M68kISelLowering.cpp
@@ -487,7 +487,7 @@
report_fatal_error("M68k interrupts may not be called directly");
auto Attr = MF.getFunction().getFnAttribute("disable-tail-calls");
- if (Attr.getValueAsString() == "true")
+ if (Attr.getValueAsBool())
IsTailCall = false;
// FIXME Add tailcalls support
diff --git a/lib/Target/Mips/MipsTargetMachine.cpp b/lib/Target/Mips/MipsTargetMachine.cpp
index 5b0b110..d4a7186 100644
--- a/lib/Target/Mips/MipsTargetMachine.cpp
+++ b/lib/Target/Mips/MipsTargetMachine.cpp
@@ -176,9 +176,7 @@
// FIXME: This is related to the code below to reset the target options,
// we need to know whether or not the soft float flag is set on the
// function, so we can enable it as a subtarget feature.
- bool softFloat =
- F.hasFnAttribute("use-soft-float") &&
- F.getFnAttribute("use-soft-float").getValueAsString() == "true";
+ bool softFloat = F.getFnAttribute("use-soft-float").getValueAsBool();
if (hasMips16Attr)
FS += FS.empty() ? "+mips16" : ",+mips16";
diff --git a/lib/Target/NVPTX/NVPTXISelLowering.cpp b/lib/Target/NVPTX/NVPTXISelLowering.cpp
index 8860e90..6a9b25f 100644
--- a/lib/Target/NVPTX/NVPTXISelLowering.cpp
+++ b/lib/Target/NVPTX/NVPTXISelLowering.cpp
@@ -4304,14 +4304,7 @@
// Allow unsafe math if unsafe-fp-math attribute explicitly says so.
const Function &F = MF.getFunction();
- if (F.hasFnAttribute("unsafe-fp-math")) {
- Attribute Attr = F.getFnAttribute("unsafe-fp-math");
- StringRef Val = Attr.getValueAsString();
- if (Val == "true")
- return true;
- }
-
- return false;
+ return F.getFnAttribute("unsafe-fp-math").getValueAsBool();
}
/// PerformADDCombineWithOperands - Try DAG combinations for an ADD with
diff --git a/lib/Target/PowerPC/PPCTargetMachine.cpp b/lib/Target/PowerPC/PPCTargetMachine.cpp
index 32b19d5..a4cfd0a 100644
--- a/lib/Target/PowerPC/PPCTargetMachine.cpp
+++ b/lib/Target/PowerPC/PPCTargetMachine.cpp
@@ -343,8 +343,7 @@
// function before we can generate a subtarget. We also need to use
// it as a key for the subtarget since that can be the only difference
// between two functions.
- bool SoftFloat =
- F.getFnAttribute("use-soft-float").getValueAsString() == "true";
+ bool SoftFloat = F.getFnAttribute("use-soft-float").getValueAsBool();
// If the soft float attribute is set on the function turn on the soft float
// subtarget feature.
if (SoftFloat)
diff --git a/lib/Target/Sparc/SparcTargetMachine.cpp b/lib/Target/Sparc/SparcTargetMachine.cpp
index ae5228d..083339b 100644
--- a/lib/Target/Sparc/SparcTargetMachine.cpp
+++ b/lib/Target/Sparc/SparcTargetMachine.cpp
@@ -117,9 +117,7 @@
// FIXME: This is related to the code below to reset the target options,
// we need to know whether or not the soft float flag is set on the
// function, so we can enable it as a subtarget feature.
- bool softFloat =
- F.hasFnAttribute("use-soft-float") &&
- F.getFnAttribute("use-soft-float").getValueAsString() == "true";
+ bool softFloat = F.getFnAttribute("use-soft-float").getValueAsBool();
if (softFloat)
FS += FS.empty() ? "+soft-float" : ",+soft-float";
diff --git a/lib/Target/SystemZ/SystemZTargetMachine.cpp b/lib/Target/SystemZ/SystemZTargetMachine.cpp
index 7b78dc4..ebb8ed9 100644
--- a/lib/Target/SystemZ/SystemZTargetMachine.cpp
+++ b/lib/Target/SystemZ/SystemZTargetMachine.cpp
@@ -179,9 +179,7 @@
// FIXME: This is related to the code below to reset the target options,
// we need to know whether or not the soft float flag is set on the
// function, so we can enable it as a subtarget feature.
- bool softFloat =
- F.hasFnAttribute("use-soft-float") &&
- F.getFnAttribute("use-soft-float").getValueAsString() == "true";
+ bool softFloat = F.getFnAttribute("use-soft-float").getValueAsBool();
if (softFloat)
FS += FS.empty() ? "+soft-float" : ",+soft-float";
diff --git a/lib/Target/TargetMachine.cpp b/lib/Target/TargetMachine.cpp
index 2aee0e5..0a655a8 100644
--- a/lib/Target/TargetMachine.cpp
+++ b/lib/Target/TargetMachine.cpp
@@ -56,7 +56,7 @@
void TargetMachine::resetTargetOptions(const Function &F) const {
#define RESET_OPTION(X, Y) \
do { \
- Options.X = (F.getFnAttribute(Y).getValueAsString() == "true"); \
+ Options.X = F.getFnAttribute(Y).getValueAsBool(); \
} while (0)
RESET_OPTION(UnsafeFPMath, "unsafe-fp-math");
diff --git a/lib/Target/X86/X86TargetMachine.cpp b/lib/Target/X86/X86TargetMachine.cpp
index 32b90e3..ff99186 100644
--- a/lib/Target/X86/X86TargetMachine.cpp
+++ b/lib/Target/X86/X86TargetMachine.cpp
@@ -294,8 +294,7 @@
// function before we can generate a subtarget. We also need to use
// it as a key for the subtarget since that can be the only difference
// between two functions.
- bool SoftFloat =
- F.getFnAttribute("use-soft-float").getValueAsString() == "true";
+ bool SoftFloat = F.getFnAttribute("use-soft-float").getValueAsBool();
// If the soft float attribute is set on the function turn on the soft float
// subtarget feature.
if (SoftFloat)
diff --git a/lib/Transforms/Instrumentation/MemorySanitizer.cpp b/lib/Transforms/Instrumentation/MemorySanitizer.cpp
index 64574a6..3e4fae5 100644
--- a/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ b/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -5028,7 +5028,7 @@
void visitCallBase(CallBase &CB, IRBuilder<> &IRB) override {
bool IsSoftFloatABI = CB.getCalledFunction()
->getFnAttribute("use-soft-float")
- .getValueAsString() == "true";
+ .getValueAsBool();
unsigned GpOffset = SystemZGpOffset;
unsigned FpOffset = SystemZFpOffset;
unsigned VrIndex = 0;
diff --git a/lib/Transforms/Scalar/TailRecursionElimination.cpp b/lib/Transforms/Scalar/TailRecursionElimination.cpp
index 8cc649a..801c9ef 100644
--- a/lib/Transforms/Scalar/TailRecursionElimination.cpp
+++ b/lib/Transforms/Scalar/TailRecursionElimination.cpp
@@ -799,7 +799,7 @@
AliasAnalysis *AA,
OptimizationRemarkEmitter *ORE,
DomTreeUpdater &DTU) {
- if (F.getFnAttribute("disable-tail-calls").getValueAsString() == "true")
+ if (F.getFnAttribute("disable-tail-calls").getValueAsBool())
return false;
bool MadeChange = false;
diff --git a/lib/Transforms/Utils/SimplifyCFG.cpp b/lib/Transforms/Utils/SimplifyCFG.cpp
index 9fdcb76..cb98227 100644
--- a/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -5793,7 +5793,7 @@
// Only build lookup table when we have a target that supports it or the
// attribute is not set.
if (!TTI.shouldBuildLookupTables() ||
- (Fn->getFnAttribute("no-jump-tables").getValueAsString() == "true"))
+ (Fn->getFnAttribute("no-jump-tables").getValueAsBool()))
return false;
// FIXME: If the switch is too sparse for a lookup table, perhaps we could
diff --git a/test/Verifier/invalid-strbool-attr.ll b/test/Verifier/invalid-strbool-attr.ll
new file mode 100644
index 0000000..672c9e4
--- /dev/null
+++ b/test/Verifier/invalid-strbool-attr.ll
@@ -0,0 +1,9 @@
+; RUN: not llvm-as < %s -o /dev/null 2>&1 | FileCheck %s
+
+; CHECK: invalid value for 'no-jump-tables' attribute: yes
+
+define void @func() #0 {
+ ret void
+}
+
+attributes #0 = { "no-jump-tables"="yes" }
