Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Clang warning diagnostics #835

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix Clang warning diagnostics #835

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)
project(clspv)
endif()

set(CMAKE_CXX_STANDARD 14)
set(CMAKE_CXX_STANDARD 17)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

/Users/jgavris/code/clspv/lib/ReplaceOpenCLBuiltinPass.cpp:3346:25: warning: hexadecimal floating literals are a C++17 feature [-Wc++17-extensions]
    const double k_pi = 0x1.921fb54442d18p+1;


# Make sure CMAKE_BUILD_TYPE gets a value because we rely on it for
# running unit tests correctly on Windows.
Expand Down Expand Up @@ -159,7 +159,7 @@ endif()
if(${CMAKE_CXX_COMPILER_ID} MATCHES "GNU")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Werror=unused-variable -Werror=switch")
elseif(${CMAKE_CXX_COMPILER_ID} MATCHES "Clang")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Werror-unused-variable -Werror-switch")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Werror=unused-variable -Werror=switch -Werror=implicit-fallthrough")
endif()

# Bring in our cmake folder
Expand Down
11 changes: 2 additions & 9 deletions lib/ArgKind.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,6 @@ using namespace llvm;

namespace {

// Maps an LLVM type for a kernel argument to an argument kind.
clspv::ArgKind GetArgKindForType(Type *type);

// Maps an LLVM type for a kernel argument to an argument
// kind suitable for embedded reflection. The result is one of:
// buffer - storage buffer
Expand All @@ -48,10 +45,6 @@ clspv::ArgKind GetArgKindForType(Type *type);
// ro_image - sampled image
// wo_image - storage image
// sampler - sampler
inline const char *GetArgKindNameForType(llvm::Type *type) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No callers

return GetArgKindName(GetArgKindForType(type));
}

clspv::ArgKind GetArgKindForType(Type *type) {
if (type->isPointerTy()) {
if (clspv::IsSamplerType(type)) {
Expand Down Expand Up @@ -89,8 +82,8 @@ clspv::ArgKind GetArgKindForType(Type *type) {
else
return clspv::ArgKind::Pod;
}
errs() << "Unhandled case in clspv::GetArgKindNameForType: " << *type << "\n";
llvm_unreachable("Unhandled case in clspv::GetArgKindNameForType");
errs() << "Unhandled case in clspv::GetArgKindForType: " << *type << "\n";
llvm_unreachable("Unhandled case in clspv::GetArgKindForType");
return clspv::ArgKind::Buffer;
}
} // namespace
Expand Down
3 changes: 3 additions & 0 deletions lib/InlineFuncWithPointerBitCastArgPass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ bool InlineFuncWithPointerBitCastArgPass::InlineFunctions(Module &M) {
// We found a call instruction which needs to be inlined!
WorkList.insert(cast<CallInst>(Inst));
// Fall-through
[[clang::fallthrough]];
case Instruction::PHI:
// If we previously checked this phi...
if (0 < CheckedPhis.count(Inst)) {
Expand All @@ -100,7 +101,9 @@ bool InlineFuncWithPointerBitCastArgPass::InlineFunctions(Module &M) {

CheckedPhis.insert(Inst);
// Fall-through
[[clang::fallthrough]];
case Instruction::GetElementPtr:
[[clang::fallthrough]];
case Instruction::BitCast:
// These pointer users could have a call user, and so we
// must check them also.
Expand Down
3 changes: 2 additions & 1 deletion lib/ReplaceOpenCLBuiltinPass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1933,6 +1933,7 @@ bool ReplaceOpenCLBuiltinPass::replaceVloadHalf(Function &F,
return replaceVloadHalf(F);
}
// Fall-through
[[clang::fallthrough]];
default:
llvm_unreachable("Unsupported vload_half vector size");
break;
Expand Down Expand Up @@ -3240,7 +3241,7 @@ bool ReplaceOpenCLBuiltinPass::replaceOrdered(Function &F, bool is_ordered) {
}

bool ReplaceOpenCLBuiltinPass::replaceIsNormal(Function &F) {
return replaceCallsWithValue(F, [this](CallInst *Call) {
return replaceCallsWithValue(F, [](CallInst *Call) {
auto ty = Call->getType();
auto x = Call->getArgOperand(0);
unsigned width = x->getType()->getScalarSizeInBits();
Expand Down
17 changes: 13 additions & 4 deletions lib/SPIRVProducerPass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -871,9 +871,11 @@ void SPIRVProducerPass::outputHeader() {
case SPIRVVersion::SPIRV_1_6:
minor = 6;
break;
#if !defined(__clang__)
default:
llvm_unreachable("unhandled spir-v version");
break;
#endif
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For Clang / LLVM, these probably don't make sense anymore as they are covered, if the warnings are kept as errors.

}
uint32_t version = (1 << 16) | (minor << 8);
binaryOut->write(reinterpret_cast<const char *>(&version), sizeof(version));
Expand Down Expand Up @@ -1216,6 +1218,7 @@ void SPIRVProducerPass::FindTypesForResourceVars() {
for (auto *elem_ty : cast<StructType>(type)->elements()) {
work_list.push_back(elem_ty);
}
break;
default:
// This type and its contained types don't get layout.
break;
Expand Down Expand Up @@ -1330,8 +1333,10 @@ SPIRVProducerPass::GetStorageClassForArgKind(clspv::ArgKind arg_kind) const {
case clspv::ArgKind::StorageImage:
case clspv::ArgKind::Sampler:
return spv::StorageClassUniformConstant;
#if !defined(__clang__)
default:
llvm_unreachable("Unsupported storage class for argument kind");
#endif
}
}

Expand Down Expand Up @@ -3323,8 +3328,8 @@ SPIRVProducerPass::GenerateImageInstruction(CallInst *Call,
const FunctionInfo &FuncInfo) {
SPIRVID RID;

auto GetExtendMask = [this](Type *sample_type,
bool is_int_image) -> uint32_t {
auto GetExtendMask = [](Type *sample_type,
bool is_int_image) -> uint32_t {
if (SpvVersion() >= SPIRVVersion::SPIRV_1_4 &&
sample_type->getScalarType()->isIntegerTy()) {
if (is_int_image)
Expand Down Expand Up @@ -4010,7 +4015,7 @@ SPIRVID SPIRVProducerPass::GenerateInstructionFromCall(CallInst *Call) {
// Generate one more instruction that uses the result of the extended
// instruction. Its result id is one more than the id of the
// extended instruction.
auto generate_extra_inst = [this, &Context, &Call,
auto generate_extra_inst = [this, &Call,
&RID](spv::Op opcode, Constant *constant) {
//
// Generate instruction like:
Expand Down Expand Up @@ -4974,7 +4979,7 @@ void SPIRVProducerPass::HandleDeferredInstruction() {
SPIRVInstruction *Placeholder = DeferredInsts[i].second;
SPIRVOperandVec Operands;

auto nextDeferred = [&i, &Inst, &DeferredInsts, &Placeholder]() {
auto nextDeferred = [&i, &DeferredInsts, &Placeholder]() {
++i;
assert(DeferredInsts.size() > i);
assert(Inst == DeferredInsts[i].first);
Expand Down Expand Up @@ -5456,10 +5461,12 @@ void SPIRVProducerPass::WriteWordCountAndOpcode(const SPIRVInstruction &Inst) {
void SPIRVProducerPass::WriteOperand(const SPIRVOperand &Op) {
SPIRVOperandType OpTy = Op.getType();
switch (OpTy) {
#if !defined(__clang__)
default: {
llvm_unreachable("Unsupported SPIRV Operand Type???");
break;
}
#endif
case SPIRVOperandType::NUMBERID: {
WriteOneWord(Op.getNumID());
break;
Expand Down Expand Up @@ -6501,9 +6508,11 @@ void SPIRVProducerPass::AddArgumentReflection(
case clspv::ArgKind::Sampler:
ext_inst = reflection::ExtInstArgumentSampler;
break;
#if !defined(__clang__)
default:
llvm_unreachable("Unhandled argument reflection");
break;
#endif
}
Ops << ext_inst << kernel_decl << getSPIRVInt32Constant(ordinal);

Expand Down