[CIR][NFC] Fix warnings in ClangIR code #133134
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This fixes unused variable warnings that have crept into the ClangIR code. In some cases the variable will be needed later, but all unused variables are being removed here. They can be reintroduced when they are needed.
andykaylor
requested review from
bcardosolopes,
erichkeane,
lanza,
AmrDeveloper and
dkolsen-pgi
llvmbot
added
clang
|
@llvm/pr-subscribers-clangir Author: Andy Kaylor (andykaylor) ChangesThis fixes unused variable warnings that have crept into the ClangIR code. In some cases the variable will be needed later, but all unused variables are being removed here. They can be reintroduced when they are needed. Full diff: https://github.com/llvm/llvm-project/pull/133134.diff 5 Files Affected:
diff --git a/clang/lib/CIR/CodeGen/CIRGenExpr.cpp b/clang/lib/CIR/CodeGen/CIRGenExpr.cpp
index 8fd09b4cfefeb..f01e03a89981d 100644
--- a/clang/lib/CIR/CodeGen/CIRGenExpr.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenExpr.cpp
@@ -170,7 +170,7 @@ mlir::Value CIRGenFunction::evaluateExprAsBool(const Expr *e) {
SourceLocation loc = e->getExprLoc();
assert(!cir::MissingFeatures::pgoUse());
- if (const MemberPointerType *MPT = e->getType()->getAs<MemberPointerType>()) {
+ if (e->getType()->getAs<MemberPointerType>()) {
cgm.errorNYI(e->getSourceRange(),
"evaluateExprAsBool: member pointer type");
return createDummyValue(getLoc(loc), boolTy);
diff --git a/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp b/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp
index 52bd3b2933744..2cf92dfbf3a5b 100644
--- a/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp
@@ -1366,8 +1366,7 @@ mlir::Value ScalarExprEmitter::VisitUnaryExprOrTypeTraitExpr(
const mlir::Location loc = cgf.getLoc(e->getSourceRange());
if (auto kind = e->getKind();
kind == UETT_SizeOf || kind == UETT_DataSizeOf) {
- if (const VariableArrayType *variableArrTy =
- cgf.getContext().getAsVariableArrayType(typeToSize)) {
+ if (cgf.getContext().getAsVariableArrayType(typeToSize)) {
cgf.getCIRGenModule().errorNYI(e->getSourceRange(),
"sizeof operator for VariableArrayType",
e->getStmtClassName());
diff --git a/clang/lib/CIR/CodeGen/CIRGenFunction.h b/clang/lib/CIR/CodeGen/CIRGenFunction.h
index cc04610f23fcb..99cc20126e360 100644
--- a/clang/lib/CIR/CodeGen/CIRGenFunction.h
+++ b/clang/lib/CIR/CodeGen/CIRGenFunction.h
@@ -367,10 +367,6 @@ class CIRGenFunction : public CIRGenTypeCache {
// class is upstreamed.
CIRGenFunction &cgf;
- // Block containing cleanup code for things initialized in this lexical
- // context (scope).
- mlir::Block *cleanupBlock = nullptr;
-
// Points to the scope entry block. This is useful, for instance, for
// helping to insert allocas before finalizing any recursive CodeGen from
// switches.
diff --git a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
index a16840cc6bfef..69303a41f2246 100644
--- a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
+++ b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
@@ -126,21 +126,6 @@ static mlir::Value emitToMemory(mlir::ConversionPatternRewriter &rewriter,
return value;
}
-static mlir::Value
-emitCirAttrToMemory(mlir::Operation *parentOp, mlir::Attribute attr,
- mlir::ConversionPatternRewriter &rewriter,
- const mlir::TypeConverter *converter,
- mlir::DataLayout const &dataLayout) {
-
- mlir::Value loweredValue =
- lowerCirAttrAsValue(parentOp, attr, rewriter, converter);
- if (auto boolAttr = mlir::dyn_cast<cir::BoolAttr>(attr)) {
- return emitToMemory(rewriter, dataLayout, boolAttr.getType(), loweredValue);
- }
-
- return loweredValue;
-}
-
mlir::LLVM::Linkage convertLinkage(cir::GlobalLinkageKind linkage) {
using CIR = cir::GlobalLinkageKind;
using LLVM = mlir::LLVM::Linkage;
@@ -261,7 +246,7 @@ mlir::Value CIRAttrToValue::visitCirAttr(cir::ConstArrayAttr attr) {
mlir::Location loc = parentOp->getLoc();
mlir::Value result;
- if (auto zeros = attr.getTrailingZerosNum()) {
+ if (attr.hasTrailingZeros()) {
mlir::Type arrayTy = attr.getType();
result = rewriter.create<mlir::LLVM::ZeroOp>(
loc, converter->convertType(arrayTy));
@@ -1251,13 +1236,12 @@ void ConvertCIRToLLVMPass::runOnOperation() {
patterns.add<CIRToLLVMStoreOpLowering>(converter, patterns.getContext(), dl);
patterns.add<CIRToLLVMGlobalOpLowering>(converter, patterns.getContext(), dl);
patterns.add<CIRToLLVMCastOpLowering>(converter, patterns.getContext(), dl);
- patterns.add<CIRToLLVMConstantOpLowering>(converter, patterns.getContext(),
- dl);
patterns.add<
// clang-format off
CIRToLLVMBinOpLowering,
CIRToLLVMBrCondOpLowering,
CIRToLLVMBrOpLowering,
+ CIRToLLVMConstantOpLowering,
CIRToLLVMFuncOpLowering,
CIRToLLVMTrapOpLowering,
CIRToLLVMUnaryOpLowering
diff --git a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.h b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.h
index ef0bb2deaccdf..b2926e75d1303 100644
--- a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.h
+++ b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.h
@@ -113,13 +113,10 @@ class CIRToLLVMStoreOpLowering
class CIRToLLVMConstantOpLowering
: public mlir::OpConversionPattern<cir::ConstantOp> {
- mlir::DataLayout const &dataLayout;
-
public:
CIRToLLVMConstantOpLowering(const mlir::TypeConverter &typeConverter,
- mlir::MLIRContext *context,
- mlir::DataLayout const &dataLayout)
- : OpConversionPattern(typeConverter, context), dataLayout(dataLayout) {
+ mlir::MLIRContext *context)
+ : OpConversionPattern(typeConverter, context) {
setHasBoundedRewriteRecursion();
}
|
|
@llvm/pr-subscribers-clang Author: Andy Kaylor (andykaylor) ChangesThis fixes unused variable warnings that have crept into the ClangIR code. In some cases the variable will be needed later, but all unused variables are being removed here. They can be reintroduced when they are needed. Full diff: https://github.com/llvm/llvm-project/pull/133134.diff 5 Files Affected:
diff --git a/clang/lib/CIR/CodeGen/CIRGenExpr.cpp b/clang/lib/CIR/CodeGen/CIRGenExpr.cpp
index 8fd09b4cfefeb..f01e03a89981d 100644
--- a/clang/lib/CIR/CodeGen/CIRGenExpr.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenExpr.cpp
@@ -170,7 +170,7 @@ mlir::Value CIRGenFunction::evaluateExprAsBool(const Expr *e) {
SourceLocation loc = e->getExprLoc();
assert(!cir::MissingFeatures::pgoUse());
- if (const MemberPointerType *MPT = e->getType()->getAs<MemberPointerType>()) {
+ if (e->getType()->getAs<MemberPointerType>()) {
cgm.errorNYI(e->getSourceRange(),
"evaluateExprAsBool: member pointer type");
return createDummyValue(getLoc(loc), boolTy);
diff --git a/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp b/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp
index 52bd3b2933744..2cf92dfbf3a5b 100644
--- a/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp
@@ -1366,8 +1366,7 @@ mlir::Value ScalarExprEmitter::VisitUnaryExprOrTypeTraitExpr(
const mlir::Location loc = cgf.getLoc(e->getSourceRange());
if (auto kind = e->getKind();
kind == UETT_SizeOf || kind == UETT_DataSizeOf) {
- if (const VariableArrayType *variableArrTy =
- cgf.getContext().getAsVariableArrayType(typeToSize)) {
+ if (cgf.getContext().getAsVariableArrayType(typeToSize)) {
cgf.getCIRGenModule().errorNYI(e->getSourceRange(),
"sizeof operator for VariableArrayType",
e->getStmtClassName());
diff --git a/clang/lib/CIR/CodeGen/CIRGenFunction.h b/clang/lib/CIR/CodeGen/CIRGenFunction.h
index cc04610f23fcb..99cc20126e360 100644
--- a/clang/lib/CIR/CodeGen/CIRGenFunction.h
+++ b/clang/lib/CIR/CodeGen/CIRGenFunction.h
@@ -367,10 +367,6 @@ class CIRGenFunction : public CIRGenTypeCache {
// class is upstreamed.
CIRGenFunction &cgf;
- // Block containing cleanup code for things initialized in this lexical
- // context (scope).
- mlir::Block *cleanupBlock = nullptr;
-
// Points to the scope entry block. This is useful, for instance, for
// helping to insert allocas before finalizing any recursive CodeGen from
// switches.
diff --git a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
index a16840cc6bfef..69303a41f2246 100644
--- a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
+++ b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
@@ -126,21 +126,6 @@ static mlir::Value emitToMemory(mlir::ConversionPatternRewriter &rewriter,
return value;
}
-static mlir::Value
-emitCirAttrToMemory(mlir::Operation *parentOp, mlir::Attribute attr,
- mlir::ConversionPatternRewriter &rewriter,
- const mlir::TypeConverter *converter,
- mlir::DataLayout const &dataLayout) {
-
- mlir::Value loweredValue =
- lowerCirAttrAsValue(parentOp, attr, rewriter, converter);
- if (auto boolAttr = mlir::dyn_cast<cir::BoolAttr>(attr)) {
- return emitToMemory(rewriter, dataLayout, boolAttr.getType(), loweredValue);
- }
-
- return loweredValue;
-}
-
mlir::LLVM::Linkage convertLinkage(cir::GlobalLinkageKind linkage) {
using CIR = cir::GlobalLinkageKind;
using LLVM = mlir::LLVM::Linkage;
@@ -261,7 +246,7 @@ mlir::Value CIRAttrToValue::visitCirAttr(cir::ConstArrayAttr attr) {
mlir::Location loc = parentOp->getLoc();
mlir::Value result;
- if (auto zeros = attr.getTrailingZerosNum()) {
+ if (attr.hasTrailingZeros()) {
mlir::Type arrayTy = attr.getType();
result = rewriter.create<mlir::LLVM::ZeroOp>(
loc, converter->convertType(arrayTy));
@@ -1251,13 +1236,12 @@ void ConvertCIRToLLVMPass::runOnOperation() {
patterns.add<CIRToLLVMStoreOpLowering>(converter, patterns.getContext(), dl);
patterns.add<CIRToLLVMGlobalOpLowering>(converter, patterns.getContext(), dl);
patterns.add<CIRToLLVMCastOpLowering>(converter, patterns.getContext(), dl);
- patterns.add<CIRToLLVMConstantOpLowering>(converter, patterns.getContext(),
- dl);
patterns.add<
// clang-format off
CIRToLLVMBinOpLowering,
CIRToLLVMBrCondOpLowering,
CIRToLLVMBrOpLowering,
+ CIRToLLVMConstantOpLowering,
CIRToLLVMFuncOpLowering,
CIRToLLVMTrapOpLowering,
CIRToLLVMUnaryOpLowering
diff --git a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.h b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.h
index ef0bb2deaccdf..b2926e75d1303 100644
--- a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.h
+++ b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.h
@@ -113,13 +113,10 @@ class CIRToLLVMStoreOpLowering
class CIRToLLVMConstantOpLowering
: public mlir::OpConversionPattern<cir::ConstantOp> {
- mlir::DataLayout const &dataLayout;
-
public:
CIRToLLVMConstantOpLowering(const mlir::TypeConverter &typeConverter,
- mlir::MLIRContext *context,
- mlir::DataLayout const &dataLayout)
- : OpConversionPattern(typeConverter, context), dataLayout(dataLayout) {
+ mlir::MLIRContext *context)
+ : OpConversionPattern(typeConverter, context) {
setHasBoundedRewriteRecursion();
}
|
mmha
approved these changes
Mar 26, 2025
| @@ -170,7 +170,7 @@ mlir::Value CIRGenFunction::evaluateExprAsBool(const Expr *e) { | |||
| SourceLocation loc = e->getExprLoc(); | |||
|
|
|||
| assert(!cir::MissingFeatures::pgoUse()); | |||
| if (const MemberPointerType *MPT = e->getType()->getAs<MemberPointerType>()) { | |||
| if (e->getType()->getAs<MemberPointerType>()) { | |||
There was a problem hiding this comment.
I wonder why getAs is not [[nodiscard]] but hey it's not so no new warning here.
(Maybe isa is still better?)
There was a problem hiding this comment.
Eventually we'll need the return value here, but probably not for a while. This isn't implemented in the incubator either.
| @@ -1366,8 +1366,7 @@ mlir::Value ScalarExprEmitter::VisitUnaryExprOrTypeTraitExpr( | |||
| const mlir::Location loc = cgf.getLoc(e->getSourceRange()); | |||
| if (auto kind = e->getKind(); | |||
| kind == UETT_SizeOf || kind == UETT_DataSizeOf) { | |||
| if (const VariableArrayType *variableArrTy = | |||
| cgf.getContext().getAsVariableArrayType(typeToSize)) { | |||
| if (cgf.getContext().getAsVariableArrayType(typeToSize)) { | |||
There was a problem hiding this comment.
This one is implemented in the incubator, and we'll use the return type when the implementation is upstreamed.
| patterns.add< | ||
| // clang-format off | ||
| CIRToLLVMBinOpLowering, | ||
| CIRToLLVMBrCondOpLowering, | ||
| CIRToLLVMBrOpLowering, | ||
| CIRToLLVMConstantOpLowering, |
There was a problem hiding this comment.
The comment right above says we'd like to keep these patterns separate because their signature is going to change in the future. That's still the case here.
There was a problem hiding this comment.
I looked at the implementation of this one in the incubator, and I don't think the dataLayout use there is necessary. I'm not sure it's necessary anywhere, since we can get it from the module. It seems reasonable to me not to pass dataLayout to this class.
bcardosolopes
approved these changes
Mar 26, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes unused variable warnings that have crept into the ClangIR code. In some cases the variable will be needed later, but all unused variables are being removed here. They can be reintroduced when they are needed.