This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
serge-sans-paille marked an inline comment as done.
Closed by commit rGd6de1e1a7140: Normalize interaction with boolean attributes
(authored by serge-sans-paille).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D99299/new/
https://reviews.llvm.org/D99299
Files:
clang/lib/CodeGen/CodeGenFunction.cpp
llvm/include/llvm/CodeGen/TargetLowering.h
llvm/include/llvm/IR/Attributes.h
llvm/lib/Analysis/IVDescriptors.cpp
llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
llvm/lib/IR/AttributeImpl.h
llvm/lib/IR/Attributes.cpp
llvm/lib/IR/Verifier.cpp
llvm/lib/Target/AMDGPU/AMDGPUCodeGenPrepare.cpp
llvm/lib/Target/AMDGPU/AMDGPULibCalls.cpp
llvm/lib/Target/AMDGPU/AMDGPULowerKernelAttributes.cpp
llvm/lib/Target/AMDGPU/AMDGPUMachineFunction.cpp
llvm/lib/Target/ARM/ARMTargetMachine.cpp
llvm/lib/Target/Hexagon/HexagonTargetMachine.cpp
llvm/lib/Target/M68k/M68kISelLowering.cpp
llvm/lib/Target/Mips/MipsTargetMachine.cpp
llvm/lib/Target/NVPTX/NVPTXISelLowering.cpp
llvm/lib/Target/PowerPC/PPCTargetMachine.cpp
llvm/lib/Target/Sparc/SparcTargetMachine.cpp
llvm/lib/Target/SystemZ/SystemZTargetMachine.cpp
llvm/lib/Target/TargetMachine.cpp
llvm/lib/Target/X86/X86TargetMachine.cpp
llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp
llvm/lib/Transforms/Utils/SimplifyCFG.cpp
llvm/test/Verifier/invalid-strbool-attr.ll
Index: llvm/test/Verifier/invalid-strbool-attr.ll
===================================================================
--- /dev/null
+++ llvm/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" }
Index: llvm/lib/Transforms/Utils/SimplifyCFG.cpp
===================================================================
--- llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ llvm/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
Index: llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp
===================================================================
--- llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp
+++ llvm/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;
Index: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
===================================================================
--- llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ llvm/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;
Index: llvm/lib/Target/X86/X86TargetMachine.cpp
===================================================================
--- llvm/lib/Target/X86/X86TargetMachine.cpp
+++ llvm/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)
Index: llvm/lib/Target/TargetMachine.cpp
===================================================================
--- llvm/lib/Target/TargetMachine.cpp
+++ llvm/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");
Index: llvm/lib/Target/SystemZ/SystemZTargetMachine.cpp
===================================================================
--- llvm/lib/Target/SystemZ/SystemZTargetMachine.cpp
+++ llvm/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";
Index: llvm/lib/Target/Sparc/SparcTargetMachine.cpp
===================================================================
--- llvm/lib/Target/Sparc/SparcTargetMachine.cpp
+++ llvm/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";
Index: llvm/lib/Target/PowerPC/PPCTargetMachine.cpp
===================================================================
--- llvm/lib/Target/PowerPC/PPCTargetMachine.cpp
+++ llvm/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)
Index: llvm/lib/Target/NVPTX/NVPTXISelLowering.cpp
===================================================================
--- llvm/lib/Target/NVPTX/NVPTXISelLowering.cpp
+++ llvm/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
Index: llvm/lib/Target/Mips/MipsTargetMachine.cpp
===================================================================
--- llvm/lib/Target/Mips/MipsTargetMachine.cpp
+++ llvm/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";
Index: llvm/lib/Target/M68k/M68kISelLowering.cpp
===================================================================
--- llvm/lib/Target/M68k/M68kISelLowering.cpp
+++ llvm/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
Index: llvm/lib/Target/Hexagon/HexagonTargetMachine.cpp
===================================================================
--- llvm/lib/Target/Hexagon/HexagonTargetMachine.cpp
+++ llvm/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];
Index: llvm/lib/Target/ARM/ARMTargetMachine.cpp
===================================================================
--- llvm/lib/Target/ARM/ARMTargetMachine.cpp
+++ llvm/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)
Index: llvm/lib/Target/AMDGPU/AMDGPUMachineFunction.cpp
===================================================================
--- llvm/lib/Target/AMDGPU/AMDGPUMachineFunction.cpp
+++ llvm/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)
Index: llvm/lib/Target/AMDGPU/AMDGPULowerKernelAttributes.cpp
===================================================================
--- llvm/lib/Target/AMDGPU/AMDGPULowerKernelAttributes.cpp
+++ llvm/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;
Index: llvm/lib/Target/AMDGPU/AMDGPULibCalls.cpp
===================================================================
--- llvm/lib/Target/AMDGPU/AMDGPULibCalls.cpp
+++ llvm/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 {
Index: llvm/lib/Target/AMDGPU/AMDGPUCodeGenPrepare.cpp
===================================================================
--- llvm/lib/Target/AMDGPU/AMDGPUCodeGenPrepare.cpp
+++ llvm/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,
Index: llvm/lib/IR/Verifier.cpp
===================================================================
--- llvm/lib/IR/Verifier.cpp
+++ llvm/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())) {
Index: llvm/lib/IR/Attributes.cpp
===================================================================
--- llvm/lib/IR/Attributes.cpp
+++ llvm/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();
Index: llvm/lib/IR/AttributeImpl.h
===================================================================
--- llvm/lib/IR/AttributeImpl.h
+++ llvm/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;
Index: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
===================================================================
--- llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
+++ llvm/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
Index: llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
===================================================================
--- llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
+++ llvm/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;
Index: llvm/lib/Analysis/IVDescriptors.cpp
===================================================================
--- llvm/lib/Analysis/IVDescriptors.cpp
+++ llvm/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");
Index: llvm/include/llvm/IR/Attributes.h
===================================================================
--- llvm/include/llvm/IR/Attributes.h
+++ llvm/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;
Index: llvm/include/llvm/CodeGen/TargetLowering.h
===================================================================
--- llvm/include/llvm/CodeGen/TargetLowering.h
+++ llvm/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) ||
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===================================================================
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -174,7 +174,7 @@
auto mergeFnAttrValue = [&](StringRef Name, bool Value) {
auto OldValue =
- CGF.CurFn->getFnAttribute(Name).getValueAsString() == "true";
+ CGF.CurFn->getFnAttribute(Name).getValueAsBool();
auto NewValue = OldValue & Value;
if (OldValue != NewValue)
CGF.CurFn->addFnAttr(Name, llvm::toStringRef(NewValue));
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
