Page MenuHomePhabricator

Please use GitHub pull requests for new patches. Phabricator shutdown timeline

Differential D118141 Diff 404850 flang/unittests/Optimizer/Builder/Runtime/RuntimeCallTestBase.h

Changeset View

Standalone View

View Options

flang/unittests/Optimizer/Builder/Runtime/RuntimeCallTestBase.h

Show First 20 Lines • Show All 114 Lines • ▼ Show 20 Linesstatic inline void checkCallOpFromResultBox(mlir::Value result,
const auto &u = result.user_begin(); const auto &u = result.user_begin();
if (mlir::isa<fir::CallOp>(*u)) if (mlir::isa<fir::CallOp>(*u))
return checkCallOp(*u, fctName, nbArgs, addLocArgs); return checkCallOp(*u, fctName, nbArgs, addLocArgs);
auto convOp = mlir::dyn_cast<fir::ConvertOp>(*u); auto convOp = mlir::dyn_cast<fir::ConvertOp>(*u);
EXPECT_NE(nullptr, convOp); EXPECT_NE(nullptr, convOp);
checkCallOpFromResultBox(convOp.getResult(), fctName, nbArgs, addLocArgs); checkCallOpFromResultBox(convOp.getResult(), fctName, nbArgs, addLocArgs);
} }
/// Check the operations in \p block for a `fir::CallOp` operation where the
/// function being called shares its function name with \p fctName and the
/// number of arguments is equal to \p nbArgs. Note that this check only cares
/// if the operation exists, and not the order in when the operation is called.
awarzynskiUnsubmitted
Done
ReplyInline Actions

Could you refine this a bit?

  • "Check the operations in \p block for containing a fir::CallOp operation where its name matches \p fctName" -> this suggests that the name of fir::CallOp should be fctName, but in fact it's the function that fir::CallOp calls that's matched against fctName
  • This method will only check the first instance of fir::CallOpt(fctName)and exit, right? I think that that's fine, but worth documenting too.
awarzynski: Could you refine this a bit? * "Check the operations in \p block for containing a `fir…
josh.mottley.armAuthorUnsubmitted
Done
ReplyInline Actions

Is this comment better?

josh.mottley.arm: Is this comment better?
/// This results in exiting the test as soon as the first correct instance of
/// `fir::CallOp` is found).
static inline void checkBlockForCallOp(
mlir::Block *block, llvm::StringRef fctName, unsigned nbArgs) {
assert(block && "mlir::Block given is a nullptr");
for (auto &op : block->getOperations()) {
if (auto callOp = mlir::dyn_cast<fir::CallOp>(op)) {
if (fctName == callOp.callee()->getRootReference().getValue()) {
EXPECT_EQ(nbArgs, callOp.args().size());
return;
}
}
}
FAIL() << "No calls to " << fctName << " were found!";
}
awarzynskiUnsubmitted
Done
ReplyInline Actions
/// order in when the operation is called.
static inline void checkBlockForCallOp(
mlir::Block *block, llvm::StringRef fctName, unsigned nbArgs) {
- EXPECT_TRUE(block);
- bool opFound = false;
+ assert(nullptr != block, "Missing MLIR block.");
for (auto &op : block->getOperations()) {
if (auto callOp = mlir::dyn_cast<fir::CallOp>(op)) {
- mlir::SymbolRefAttr callee = *callOp.callee();
- if (fctName == callee.getRootReference().getValue()) {
- opFound = true;
- EXPECT_EQ(nbArgs, callOp.args().size());
- break;
+ if (fctName == callOp.callee()->getRootReference().getValue()) {
+ EXPECT_EQ(nbArgs, callOp.args().size()) << "A call to `" << fctName << "` was found, but the number of arguments is incorrect";
+ return;
}
}
}
- EXPECT_TRUE(opFound);
+ FAIL() << "No calls to `" << fctName << "` were found!";
}
#endif // FORTRAN_OPTIMIZER_BUILDER_RUNTIME_RUNTIMECALLTESTBASE_H

A bit more context:

  • EXPECT_TRUE(block); --> I would move this to where the block is created. In this function I'd use assertinstead (to verify that this method is used correctly)
  • you don't really need opFound :)
  • everything else is a "nit" - feel free to ignore
awarzynski: A bit more context: * ` EXPECT_TRUE(block);` --> I would move this to where the block is…
#endif // FORTRAN_OPTIMIZER_BUILDER_RUNTIME_RUNTIMECALLTESTBASE_H #endif // FORTRAN_OPTIMIZER_BUILDER_RUNTIME_RUNTIMECALLTESTBASE_H