Skip to content

[CIR][NFC] Fix warnings in ClangIR code #133134

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

Merged
merged 1 commit into from
Mar 31, 2025
Merged

[CIR][NFC] Fix warnings in ClangIR code #133134

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
2 changes: 1 addition & 1 deletion clang/lib/CIR/CodeGen/CIRGenExpr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder why getAs is not [[nodiscard]] but hey it's not so no new warning here.

(Maybe isa is still better?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Eventually we'll need the return value here, but probably not for a while. This isn't implemented in the incubator either.

cgm.errorNYI(e->getSourceRange(),
"evaluateExprAsBool: member pointer type");
return createDummyValue(getLoc(loc), boolTy);
Expand Down
3 changes: 1 addition & 2 deletions clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This one is implemented in the incubator, and we'll use the return type when the implementation is upstreamed.

cgf.getCIRGenModule().errorNYI(e->getSourceRange(),
"sizeof operator for VariableArrayType",
e->getStmtClassName());
Expand Down
4 changes: 0 additions & 4 deletions clang/lib/CIR/CodeGen/CIRGenFunction.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
20 changes: 2 additions & 18 deletions clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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,
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

CIRToLLVMFuncOpLowering,
CIRToLLVMTrapOpLowering,
CIRToLLVMUnaryOpLowering
Expand Down
7 changes: 2 additions & 5 deletions clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down