From 974f0dfa07dfc39e417275b44c3f2722f80eb804 Mon Sep 17 00:00:00 2001 From: erichkeane Date: Wed, 1 Oct 2025 06:33:53 -0700 Subject: [PATCH] [OpenACC] Remove unnecessary uses of `getResult`, fix cast tests A previous review comment pointed out that operations with only a single result implicitly convert to `mlir::Value`. This patch removes the explicit use of `getResult` where it is unnecessary in OpenACC lowering. However, there ARE a few cases where it is necessary where the `mlir::ValueRange` implicit constructor from a single value is being used, so those are untouched. Additionally, while the previous patch was being committed (#161382), a second patch (#161431) changed the format of cir.casts, so this patch fixes the additional test lines for that as well. --- clang/lib/CIR/CodeGen/CIRGenOpenACC.cpp | 2 +- clang/lib/CIR/CodeGen/CIRGenOpenACCClause.cpp | 18 ++++++------- clang/lib/CIR/CodeGen/CIRGenOpenACCRecipe.cpp | 26 +++++++++---------- ...-clause-pointer-array-recipes-CtorDtor.cpp | 8 +++--- ...ate-clause-pointer-array-recipes-NoOps.cpp | 8 +++--- ...ivate-clause-pointer-array-recipes-int.cpp | 8 +++--- 6 files changed, 34 insertions(+), 36 deletions(-) diff --git a/clang/lib/CIR/CodeGen/CIRGenOpenACC.cpp b/clang/lib/CIR/CodeGen/CIRGenOpenACC.cpp index 7f9350a9e4173..a9af753381db3 100644 --- a/clang/lib/CIR/CodeGen/CIRGenOpenACC.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenOpenACC.cpp @@ -62,7 +62,7 @@ mlir::Value CIRGenFunction::createOpenACCConstantInt(mlir::Location loc, auto constOp = builder.create( loc, builder.getIntegerAttr(ty, value)); - return constOp.getResult(); + return constOp; } CIRGenFunction::OpenACCDataOperandInfo diff --git a/clang/lib/CIR/CodeGen/CIRGenOpenACCClause.cpp b/clang/lib/CIR/CodeGen/CIRGenOpenACCClause.cpp index 3cf053449458f..f22f3e8845f8a 100644 --- a/clang/lib/CIR/CodeGen/CIRGenOpenACCClause.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenOpenACCClause.cpp @@ -110,7 +110,7 @@ class OpenACCClauseCIREmitter final auto constOp = builder.create( loc, builder.getIntegerAttr(ty, value)); - return constOp.getResult(); + return constOp; } mlir::Value createConstantInt(SourceLocation loc, unsigned width, @@ -230,13 +230,13 @@ class OpenACCClauseCIREmitter final std::is_same_v) { // Detach/Delete ops don't have the variable reference here, so they // take 1 fewer argument to their build function. - afterOp = builder.create( - opInfo.beginLoc, beforeOp.getResult(), structured, implicit, - opInfo.name, opInfo.bounds); + afterOp = + builder.create(opInfo.beginLoc, beforeOp, structured, + implicit, opInfo.name, opInfo.bounds); } else { afterOp = builder.create( - opInfo.beginLoc, beforeOp.getResult(), opInfo.varValue, structured, - implicit, opInfo.name, opInfo.bounds); + opInfo.beginLoc, beforeOp, opInfo.varValue, structured, implicit, + opInfo.name, opInfo.bounds); } } @@ -1005,7 +1005,7 @@ class OpenACCClauseCIREmitter final /*temporary=*/nullptr, OpenACCReductionOperator::Invalid, Decl::castToDeclContext(cgf.curFuncDecl), opInfo.origType, opInfo.bounds.size(), opInfo.boundTypes, opInfo.baseType, - privateOp.getResult()); + privateOp); // TODO: OpenACC: The dialect is going to change in the near future to // have these be on a different operation, so when that changes, we // probably need to change these here. @@ -1053,7 +1053,7 @@ class OpenACCClauseCIREmitter final OpenACCReductionOperator::Invalid, Decl::castToDeclContext(cgf.curFuncDecl), opInfo.origType, opInfo.bounds.size(), opInfo.boundTypes, opInfo.baseType, - firstPrivateOp.getResult()); + firstPrivateOp); // TODO: OpenACC: The dialect is going to change in the near future to // have these be on a different operation, so when that changes, we @@ -1101,7 +1101,7 @@ class OpenACCClauseCIREmitter final /*temporary=*/nullptr, clause.getReductionOp(), Decl::castToDeclContext(cgf.curFuncDecl), opInfo.origType, opInfo.bounds.size(), opInfo.boundTypes, opInfo.baseType, - reductionOp.getResult()); + reductionOp); operation.addReduction(builder.getContext(), reductionOp, recipe); } diff --git a/clang/lib/CIR/CodeGen/CIRGenOpenACCRecipe.cpp b/clang/lib/CIR/CodeGen/CIRGenOpenACCRecipe.cpp index 25cacbb73eb03..e603884d08e3c 100644 --- a/clang/lib/CIR/CodeGen/CIRGenOpenACCRecipe.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenOpenACCRecipe.cpp @@ -240,7 +240,7 @@ OpenACCRecipeBuilderBase::createBoundsLoop(mlir::Value subscriptedValue, if (auto arrayTy = dyn_cast(eltTy)) return builder.getArrayElement(loc, loc, subVal, arrayTy.getElementType(), - idxLoad.getResult(), + idxLoad, /*shouldDecay=*/true); assert(isa(eltTy)); @@ -248,8 +248,8 @@ OpenACCRecipeBuilderBase::createBoundsLoop(mlir::Value subscriptedValue, auto eltLoad = cir::LoadOp::create(builder, loc, {subVal}); return cir::PtrStrideOp::create(builder, loc, eltLoad.getType(), eltLoad, - idxLoad.getResult()) - .getResult(); + idxLoad); + }; auto forStmtBuilder = [&]() { @@ -271,12 +271,11 @@ OpenACCRecipeBuilderBase::createBoundsLoop(mlir::Value subscriptedValue, if (inverse) { cir::ConstantOp constOne = builder.getConstInt(loc, itrTy, 1); - auto sub = - cir::BinOp::create(builder, loc, itrTy, cir::BinOpKind::Sub, - ubConversion.getResult(0), constOne.getResult()); + auto sub = cir::BinOp::create(builder, loc, itrTy, cir::BinOpKind::Sub, + ubConversion.getResult(0), constOne); // Upperbound is exclusive, so subtract 1. - builder.CIRBaseBuilderTy::createStore(loc, sub.getResult(), itr); + builder.CIRBaseBuilderTy::createStore(loc, sub, itr); } else { // Lowerbound is inclusive, so we can include it. builder.CIRBaseBuilderTy::createStore(loc, lbConversion.getResult(0), @@ -294,8 +293,8 @@ OpenACCRecipeBuilderBase::createBoundsLoop(mlir::Value subscriptedValue, auto loadCur = cir::LoadOp::create(builder, loc, {itr}); // Use 'not equal' since we are just doing an increment/decrement. auto cmp = builder.createCompare( - loc, inverse ? cir::CmpOpKind::ge : cir::CmpOpKind::lt, - loadCur.getResult(), endItr.getResult(0)); + loc, inverse ? cir::CmpOpKind::ge : cir::CmpOpKind::lt, loadCur, + endItr.getResult(0)); builder.createCondition(cmp); }, /*bodyBuilder=*/ @@ -309,11 +308,10 @@ OpenACCRecipeBuilderBase::createBoundsLoop(mlir::Value subscriptedValue, /*stepBuilder=*/ [&](mlir::OpBuilder &b, mlir::Location loc) { auto load = cir::LoadOp::create(builder, loc, {itr}); - auto unary = cir::UnaryOp::create(builder, loc, load.getType(), - inverse ? cir::UnaryOpKind::Dec - : cir::UnaryOpKind::Inc, - load.getResult()); - builder.CIRBaseBuilderTy::createStore(loc, unary.getResult(), itr); + auto unary = cir::UnaryOp::create( + builder, loc, load.getType(), + inverse ? cir::UnaryOpKind::Dec : cir::UnaryOpKind::Inc, load); + builder.CIRBaseBuilderTy::createStore(loc, unary, itr); builder.createYield(loc); }); }; diff --git a/clang/test/CIR/CodeGenOpenACC/private-clause-pointer-array-recipes-CtorDtor.cpp b/clang/test/CIR/CodeGenOpenACC/private-clause-pointer-array-recipes-CtorDtor.cpp index da56de005d5f2..4d0e481a9d19e 100644 --- a/clang/test/CIR/CodeGenOpenACC/private-clause-pointer-array-recipes-CtorDtor.cpp +++ b/clang/test/CIR/CodeGenOpenACC/private-clause-pointer-array-recipes-CtorDtor.cpp @@ -502,7 +502,7 @@ void do_things(unsigned A, unsigned B) { // CHECK-NEXT: %[[UB2_CAST:.*]] = builtin.unrealized_conversion_cast %[[UB2]] : index to !u64i // // CHECK-NEXT: %[[ZERO:.*]] = cir.const #cir.int<0> : !u64i -// CHECK-NEXT: %[[DECAY:.*]] = cir.cast(array_to_ptrdecay, %[[TL_ALLOCA]] : !cir.ptr x 5>>), !cir.ptr> +// CHECK-NEXT: %[[DECAY:.*]] = cir.cast array_to_ptrdecay %[[TL_ALLOCA]] : !cir.ptr x 5>> -> !cir.ptr> // CHECK-NEXT: %[[TL_DEREF:.*]] = cir.ptr_stride(%[[DECAY]] : !cir.ptr>, %[[ZERO]] : !u64i), !cir.ptr> // // CHECK-NEXT: %[[UB1:.*]] = acc.get_upperbound %[[BOUND1]] : (!acc.data_bounds_ty) -> index @@ -721,7 +721,7 @@ void do_things(unsigned A, unsigned B) { // CHECK-NEXT: %[[UB3_CAST:.*]] = builtin.unrealized_conversion_cast %[[UB3]] : index to !u64i // // CHECK-NEXT: %[[ZERO:.*]] = cir.const #cir.int<0> : !u64i -// CHECK-NEXT: %[[DECAY:.*]] = cir.cast(array_to_ptrdecay, %[[TL_ALLOCA]] : !cir.ptr> x 5>>), !cir.ptr>> +// CHECK-NEXT: %[[DECAY:.*]] = cir.cast array_to_ptrdecay %[[TL_ALLOCA]] : !cir.ptr> x 5>> -> !cir.ptr>> // CHECK-NEXT: %[[TL_DEREF:.*]] = cir.ptr_stride(%[[DECAY]] : !cir.ptr>>, %[[ZERO]] : !u64i), !cir.ptr>> // // CHECK-NEXT: %[[UB2:.*]] = acc.get_upperbound %[[BOUND2]] : (!acc.data_bounds_ty) -> index @@ -882,7 +882,7 @@ void do_things(unsigned A, unsigned B) { // CHECK-NEXT: %[[UB2_CAST:.*]] = builtin.unrealized_conversion_cast %[[UB2]] : index to !u64i // // CHECK-NEXT: %[[ZERO:.*]] = cir.const #cir.int<0> : !u64i -// CHECK-NEXT: %[[DECAY:.*]] = cir.cast(array_to_ptrdecay, %[[TL_ALLOCA]] : !cir.ptr> x 5>>), !cir.ptr>> +// CHECK-NEXT: %[[DECAY:.*]] = cir.cast array_to_ptrdecay %[[TL_ALLOCA]] : !cir.ptr> x 5>> -> !cir.ptr>> // CHECK-NEXT: %[[TL_DEREF:.*]] = cir.ptr_stride(%[[DECAY]] : !cir.ptr>>, %[[ZERO]] : !u64i), !cir.ptr>> // // CHECK-NEXT: %[[UB1:.*]] = acc.get_upperbound %[[BOUND1]] : (!acc.data_bounds_ty) -> index @@ -1281,7 +1281,7 @@ void do_things(unsigned A, unsigned B) { // CHECK-NEXT: %[[NUM_ELTS:.*]] = cir.binop(mul, %[[UB2_CAST]], %[[UB3_CAST]]) : !u64i // // CHECK-NEXT: %[[ZERO:.*]] = cir.const #cir.int<0> : !u64i -// CHECK-NEXT: %[[DECAY:.*]] = cir.cast(array_to_ptrdecay, %[[ARR_ALLOCA]] : !cir.ptr x 5>>), !cir.ptr> +// CHECK-NEXT: %[[DECAY:.*]] = cir.cast array_to_ptrdecay %[[ARR_ALLOCA]] : !cir.ptr x 5>> -> !cir.ptr> // CHECK-NEXT: %[[STRIDE:.*]] = cir.ptr_stride(%[[DECAY]] : !cir.ptr>, %[[ZERO]] : !u64i), !cir.ptr> // // CHECK-NEXT: %[[UB1:.*]] = acc.get_upperbound %[[BOUND1]] : (!acc.data_bounds_ty) -> index diff --git a/clang/test/CIR/CodeGenOpenACC/private-clause-pointer-array-recipes-NoOps.cpp b/clang/test/CIR/CodeGenOpenACC/private-clause-pointer-array-recipes-NoOps.cpp index f9230dcaf1691..4687320d3fca2 100644 --- a/clang/test/CIR/CodeGenOpenACC/private-clause-pointer-array-recipes-NoOps.cpp +++ b/clang/test/CIR/CodeGenOpenACC/private-clause-pointer-array-recipes-NoOps.cpp @@ -330,7 +330,7 @@ void do_things(unsigned A, unsigned B) { // CHECK-NEXT: %[[UB2_CAST:.*]] = builtin.unrealized_conversion_cast %[[UB2]] : index to !u64i // // CHECK-NEXT: %[[ZERO:.*]] = cir.const #cir.int<0> : !u64i -// CHECK-NEXT: %[[DECAY:.*]] = cir.cast(array_to_ptrdecay, %[[TL_ALLOCA]] : !cir.ptr x 5>>), !cir.ptr> +// CHECK-NEXT: %[[DECAY:.*]] = cir.cast array_to_ptrdecay %[[TL_ALLOCA]] : !cir.ptr x 5>> -> !cir.ptr> // CHECK-NEXT: %[[TL_DEREF:.*]] = cir.ptr_stride(%[[DECAY]] : !cir.ptr>, %[[ZERO]] : !u64i), !cir.ptr> // // CHECK-NEXT: %[[UB1:.*]] = acc.get_upperbound %[[BOUND1]] : (!acc.data_bounds_ty) -> index @@ -440,7 +440,7 @@ void do_things(unsigned A, unsigned B) { // CHECK-NEXT: %[[UB3_CAST:.*]] = builtin.unrealized_conversion_cast %[[UB3]] : index to !u64i // // CHECK-NEXT: %[[ZERO:.*]] = cir.const #cir.int<0> : !u64i -// CHECK-NEXT: %[[DECAY:.*]] = cir.cast(array_to_ptrdecay, %[[TL_ALLOCA]] : !cir.ptr> x 5>>), !cir.ptr>> +// CHECK-NEXT: %[[DECAY:.*]] = cir.cast array_to_ptrdecay %[[TL_ALLOCA]] : !cir.ptr> x 5>> -> !cir.ptr>> // CHECK-NEXT: %[[TL_DEREF:.*]] = cir.ptr_stride(%[[DECAY]] : !cir.ptr>>, %[[ZERO]] : !u64i), !cir.ptr>> // // CHECK-NEXT: %[[UB2:.*]] = acc.get_upperbound %[[BOUND2]] : (!acc.data_bounds_ty) -> index @@ -521,7 +521,7 @@ void do_things(unsigned A, unsigned B) { // CHECK-NEXT: %[[UB2_CAST:.*]] = builtin.unrealized_conversion_cast %[[UB2]] : index to !u64i // // CHECK-NEXT: %[[ZERO:.*]] = cir.const #cir.int<0> : !u64i -// CHECK-NEXT: %[[DECAY:.*]] = cir.cast(array_to_ptrdecay, %[[TL_ALLOCA]] : !cir.ptr> x 5>>), !cir.ptr>> +// CHECK-NEXT: %[[DECAY:.*]] = cir.cast array_to_ptrdecay %[[TL_ALLOCA]] : !cir.ptr> x 5>> -> !cir.ptr>> // CHECK-NEXT: %[[TL_DEREF:.*]] = cir.ptr_stride(%[[DECAY]] : !cir.ptr>>, %[[ZERO]] : !u64i), !cir.ptr>> // // CHECK-NEXT: %[[UB1:.*]] = acc.get_upperbound %[[BOUND1]] : (!acc.data_bounds_ty) -> index @@ -767,7 +767,7 @@ void do_things(unsigned A, unsigned B) { // CHECK-NEXT: %[[NUM_ELTS:.*]] = cir.binop(mul, %[[UB2_CAST]], %[[UB3_CAST]]) : !u64i // // CHECK-NEXT: %[[ZERO:.*]] = cir.const #cir.int<0> : !u64i -// CHECK-NEXT: %[[DECAY:.*]] = cir.cast(array_to_ptrdecay, %[[ARR_ALLOCA]] : !cir.ptr x 5>>), !cir.ptr> +// CHECK-NEXT: %[[DECAY:.*]] = cir.cast array_to_ptrdecay %[[ARR_ALLOCA]] : !cir.ptr x 5>> -> !cir.ptr> // CHECK-NEXT: %[[STRIDE:.*]] = cir.ptr_stride(%[[DECAY]] : !cir.ptr>, %[[ZERO]] : !u64i), !cir.ptr> // // CHECK-NEXT: %[[UB1:.*]] = acc.get_upperbound %[[BOUND1]] : (!acc.data_bounds_ty) -> index diff --git a/clang/test/CIR/CodeGenOpenACC/private-clause-pointer-array-recipes-int.cpp b/clang/test/CIR/CodeGenOpenACC/private-clause-pointer-array-recipes-int.cpp index 45d8b78a4a128..db5d5782bcecd 100644 --- a/clang/test/CIR/CodeGenOpenACC/private-clause-pointer-array-recipes-int.cpp +++ b/clang/test/CIR/CodeGenOpenACC/private-clause-pointer-array-recipes-int.cpp @@ -327,7 +327,7 @@ void do_things(unsigned A, unsigned B) { // CHECK-NEXT: %[[UB2_CAST:.*]] = builtin.unrealized_conversion_cast %[[UB2]] : index to !u64i // // CHECK-NEXT: %[[ZERO:.*]] = cir.const #cir.int<0> : !u64i -// CHECK-NEXT: %[[DECAY:.*]] = cir.cast(array_to_ptrdecay, %[[TL_ALLOCA]] : !cir.ptr x 5>>), !cir.ptr> +// CHECK-NEXT: %[[DECAY:.*]] = cir.cast array_to_ptrdecay %[[TL_ALLOCA]] : !cir.ptr x 5>> -> !cir.ptr> // CHECK-NEXT: %[[TL_DEREF:.*]] = cir.ptr_stride(%[[DECAY]] : !cir.ptr>, %[[ZERO]] : !u64i), !cir.ptr> // // CHECK-NEXT: %[[UB1:.*]] = acc.get_upperbound %[[BOUND1]] : (!acc.data_bounds_ty) -> index @@ -437,7 +437,7 @@ void do_things(unsigned A, unsigned B) { // CHECK-NEXT: %[[UB3_CAST:.*]] = builtin.unrealized_conversion_cast %[[UB3]] : index to !u64i // // CHECK-NEXT: %[[ZERO:.*]] = cir.const #cir.int<0> : !u64i -// CHECK-NEXT: %[[DECAY:.*]] = cir.cast(array_to_ptrdecay, %[[TL_ALLOCA]] : !cir.ptr> x 5>>), !cir.ptr>> +// CHECK-NEXT: %[[DECAY:.*]] = cir.cast array_to_ptrdecay %[[TL_ALLOCA]] : !cir.ptr> x 5>> -> !cir.ptr>> // CHECK-NEXT: %[[TL_DEREF:.*]] = cir.ptr_stride(%[[DECAY]] : !cir.ptr>>, %[[ZERO]] : !u64i), !cir.ptr>> // // CHECK-NEXT: %[[UB2:.*]] = acc.get_upperbound %[[BOUND2]] : (!acc.data_bounds_ty) -> index @@ -518,7 +518,7 @@ void do_things(unsigned A, unsigned B) { // CHECK-NEXT: %[[UB2_CAST:.*]] = builtin.unrealized_conversion_cast %[[UB2]] : index to !u64i // // CHECK-NEXT: %[[ZERO:.*]] = cir.const #cir.int<0> : !u64i -// CHECK-NEXT: %[[DECAY:.*]] = cir.cast(array_to_ptrdecay, %[[TL_ALLOCA]] : !cir.ptr> x 5>>), !cir.ptr>> +// CHECK-NEXT: %[[DECAY:.*]] = cir.cast array_to_ptrdecay %[[TL_ALLOCA]] : !cir.ptr> x 5>> -> !cir.ptr>> // CHECK-NEXT: %[[TL_DEREF:.*]] = cir.ptr_stride(%[[DECAY]] : !cir.ptr>>, %[[ZERO]] : !u64i), !cir.ptr>> // // CHECK-NEXT: %[[UB1:.*]] = acc.get_upperbound %[[BOUND1]] : (!acc.data_bounds_ty) -> index @@ -765,7 +765,7 @@ void do_things(unsigned A, unsigned B) { // CHECK-NEXT: %[[NUM_ELTS:.*]] = cir.binop(mul, %[[UB2_CAST]], %[[UB3_CAST]]) : !u64i // // CHECK-NEXT: %[[ZERO:.*]] = cir.const #cir.int<0> : !u64i -// CHECK-NEXT: %[[DECAY:.*]] = cir.cast(array_to_ptrdecay, %[[ARR_ALLOCA]] : !cir.ptr x 5>>), !cir.ptr> +// CHECK-NEXT: %[[DECAY:.*]] = cir.cast array_to_ptrdecay %[[ARR_ALLOCA]] : !cir.ptr x 5>> -> !cir.ptr> // CHECK-NEXT: %[[STRIDE:.*]] = cir.ptr_stride(%[[DECAY]] : !cir.ptr>, %[[ZERO]] : !u64i), !cir.ptr> // // CHECK-NEXT: %[[UB1:.*]] = acc.get_upperbound %[[BOUND1]] : (!acc.data_bounds_ty) -> index