-
Notifications
You must be signed in to change notification settings - Fork 0
[mlir][Sol] Build precise per-type dispatch tables for internal fnptr calls #141
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3952,39 +3952,64 @@ struct ICallOpLowering : public OpConversionPattern<sol::ICallOp> { | |
| // is lowered, so the dispatch table can still inspect the Sol symbol table. | ||
| using OpConversionPattern<sol::ICallOp>::OpConversionPattern; | ||
|
|
||
| LogicalResult matchAndRewrite(sol::ICallOp op, OpAdaptor adaptor, | ||
| ConversionPatternRewriter &r) const override { | ||
| /// Generates the dispatch function that switches over the candidate | ||
| /// functions of the caller's phase. Outlining this reduces the caller's | ||
| /// stack pressure. | ||
| yul::FuncOp genDispatchFn(StringRef name, ArrayRef<int64_t> candidateIds, | ||
| FunctionType calleeTy, Type calleeIdTy, | ||
| bool callerRuntime, sol::ICallOp op, | ||
| ConversionPatternRewriter &r) const { | ||
| Location loc = op.getLoc(); | ||
| Operation *symTab = SymbolTable::getNearestSymbolTable(op); | ||
| evm::Builder evmB(getModule(op), r, loc); | ||
|
|
||
| auto calleeArgs = adaptor.getOperands().drop_front(); | ||
| SmallVector<Type> convertedResTys; | ||
| if (failed(getTypeConverter()->convertTypes(op.getResultTypes(), | ||
| convertedResTys))) | ||
| return failure(); | ||
|
|
||
| auto calleeTy = mlir::FunctionType::get( | ||
| r.getContext(), calleeArgs.getTypes(), convertedResTys); | ||
|
|
||
| // Collect functions with matching signature. | ||
| Operation *symTab = SymbolTable::getNearestSymbolTable(op); | ||
| bool callerRuntime = op->getParentOfType<sol::FuncOp>().getRuntime(); | ||
| // Resolve the candidate ids in the caller's phase. | ||
| DenseSet<int64_t> candidateIdSet(candidateIds.begin(), candidateIds.end()); | ||
| SmallVector<int64_t> caseIds; | ||
| SmallVector<sol::FuncOp> caseFns; | ||
| symTab->walk([&](sol::FuncOp fn) { | ||
| // At this stage the nearest Sol symbol table can still contain both | ||
| // creation and runtime functions. Internal function pointers are only | ||
| // valid within the caller's phase, so do not dispatch across that | ||
| // boundary just because the id and signature match. | ||
| // boundary just because the id matches. | ||
| if (fn.getRuntime() != callerRuntime) | ||
| return; | ||
| if (fn.getId() && fn.getFunctionType() == calleeTy) { | ||
| if (fn.getId() && candidateIdSet.contains(*fn.getId())) { | ||
| assert(fn.getFunctionType() == calleeTy && | ||
| "candidate signature mismatch"); | ||
| caseFns.push_back(fn); | ||
| caseIds.push_back(*fn.getId()); | ||
| } | ||
| }); | ||
|
|
||
| auto calleeIntTy = cast<IntegerType>(adaptor.getCallee().getType()); | ||
| OpBuilder::InsertionGuard insertGuard(r); | ||
| r.setInsertionPointAfter(op->getParentOfType<sol::FuncOp>()); | ||
|
|
||
| SmallVector<Type> inputTys{calleeIdTy}; | ||
| llvm::append_range(inputTys, calleeTy.getInputs()); | ||
| auto fnTy = mlir::FunctionType::get(r.getContext(), inputTys, | ||
| calleeTy.getResults()); | ||
| auto fn = r.create<yul::FuncOp>(loc, name, fnTy); | ||
|
|
||
| SmallVector<Location> argLocs(inputTys.size(), loc); | ||
| Block *entry = | ||
| r.createBlock(&fn.getBody(), fn.getBody().end(), inputTys, argLocs); | ||
| r.setInsertionPointToStart(entry); | ||
| ValueRange calleeArgs = entry->getArguments().drop_front(); | ||
|
|
||
| // No function of this signature is ever taken as a value, so no valid id | ||
| // can reach this call: unconditionally panic. | ||
| if (caseFns.empty()) { | ||
| evmB.genPanic(mlir::evm::PanicCode::InvalidInternalFunction); | ||
| SmallVector<Value> undefs; | ||
| undefs.reserve(calleeTy.getNumResults()); | ||
| for (Type ty : calleeTy.getResults()) | ||
| undefs.push_back(r.create<LLVM::UndefOp>(loc, ty)); | ||
| r.create<yul::FuncReturnOp>(loc, undefs); | ||
| return fn; | ||
| } | ||
|
|
||
| auto calleeIntTy = cast<IntegerType>(calleeIdTy); | ||
| SmallVector<APInt> caseVals; | ||
| caseVals.reserve(caseIds.size()); | ||
| for (int64_t caseId : caseIds) | ||
|
|
@@ -3993,24 +4018,60 @@ struct ICallOpLowering : public OpConversionPattern<sol::ICallOp> { | |
| RankedTensorType::get(static_cast<int64_t>(caseVals.size()), | ||
| calleeIntTy), | ||
| caseVals); | ||
| auto switchOp = r.create<yul::SwitchOp>( | ||
| loc, convertedResTys, adaptor.getCallee(), caseIdsAttr, caseIds.size()); | ||
| auto switchOp = r.create<yul::SwitchOp>(loc, calleeTy.getResults(), | ||
| entry->getArgument(0), caseIdsAttr, | ||
| caseIds.size()); | ||
| for (size_t i = 0; i < caseFns.size(); ++i) { | ||
| r.setInsertionPointToStart(&switchOp.getCaseRegions()[i].emplaceBlock()); | ||
| auto call = r.create<yul::FuncCallOp>( | ||
| loc, convertedResTys, FlatSymbolRefAttr::get(caseFns[i]), calleeArgs); | ||
| auto call = r.create<yul::FuncCallOp>(loc, calleeTy.getResults(), | ||
| FlatSymbolRefAttr::get(caseFns[i]), | ||
| calleeArgs); | ||
| r.create<yul::YieldOp>(loc, call.getResults()); | ||
| } | ||
|
|
||
| r.setInsertionPointToStart(&switchOp.getDefaultRegion().emplaceBlock()); | ||
| evmB.genPanic(mlir::evm::PanicCode::InvalidInternalFunction); | ||
| SmallVector<Value> undefs; | ||
| undefs.reserve(op.getNumResults()); | ||
| for (Type ty : convertedResTys) | ||
| undefs.reserve(calleeTy.getNumResults()); | ||
| for (Type ty : calleeTy.getResults()) | ||
| undefs.push_back(r.create<LLVM::UndefOp>(loc, ty)); | ||
| r.create<yul::YieldOp>(loc, undefs); | ||
|
|
||
| r.replaceOp(op, switchOp.getResults()); | ||
| r.setInsertionPointToEnd(entry); | ||
| r.create<yul::FuncReturnOp>(loc, switchOp.getResults()); | ||
| return fn; | ||
| } | ||
|
|
||
| LogicalResult matchAndRewrite(sol::ICallOp op, OpAdaptor adaptor, | ||
| ConversionPatternRewriter &r) const override { | ||
| auto calleeArgs = adaptor.getOperands().drop_front(); | ||
| SmallVector<Type> convertedResTys; | ||
| if (failed(getTypeConverter()->convertTypes(op.getResultTypes(), | ||
| convertedResTys))) | ||
| return failure(); | ||
|
|
||
| auto calleeTy = mlir::FunctionType::get( | ||
| r.getContext(), calleeArgs.getTypes(), convertedResTys); | ||
|
|
||
| // Set at pass entry, before the sol signatures are legalized. | ||
| std::optional<ArrayRef<int64_t>> candidateIds = op.getCandidates(); | ||
| assert(candidateIds && "sol.icall without candidate annotation"); | ||
|
Comment on lines
+4057
to
+4058
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks like we have here an API mismatch: in TableGen candidates are declared as optional, |
||
|
|
||
| Operation *symTab = SymbolTable::getNearestSymbolTable(op); | ||
| bool callerRuntime = op->getParentOfType<sol::FuncOp>().getRuntime(); | ||
| std::string dispatchFnName = | ||
| evm::helpersym::internalDispatch(callerRuntime, *candidateIds); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks like we may get a symbol collision for empty candidateIds. For example: causes a crash, as it produces one func.func @__sol.internal_dispatch.rt : (i256, i256) -> i256 and two incompatible call sites. I'd suggest to inline an empty dispatch function (that contains only Panic(0x51) call). |
||
| auto dispatchFn = dyn_cast_or_null<yul::FuncOp>( | ||
| SymbolTable::lookupSymbolIn(symTab, dispatchFnName)); | ||
|
Comment on lines
+4064
to
+4065
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd suggest to add here more checks that we do not cause symbol clashing with:
|
||
| if (!dispatchFn) | ||
| dispatchFn = | ||
| genDispatchFn(dispatchFnName, *candidateIds, calleeTy, | ||
| adaptor.getCallee().getType(), callerRuntime, op, r); | ||
|
|
||
| SmallVector<Value> callArgs{adaptor.getCallee()}; | ||
| llvm::append_range(callArgs, calleeArgs); | ||
| r.replaceOpWithNewOp<yul::FuncCallOp>( | ||
| op, convertedResTys, FlatSymbolRefAttr::get(dispatchFn), callArgs); | ||
| return success(); | ||
| } | ||
| }; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,6 +16,7 @@ | |
| #include "mlir/Dialect/Sol/Sol.h" | ||
| #include "mlir/Dialect/Yul/Yul.h" | ||
| #include "mlir/Transforms/DialectConversion.h" | ||
| #include "llvm/ADT/SetVector.h" | ||
|
|
||
| namespace mlir { | ||
| #define GEN_PASS_DEF_CONVERTSOLTOYULPASS | ||
|
|
@@ -295,9 +296,45 @@ struct ConvertSolToYulPass | |
| return success(); | ||
| } | ||
|
|
||
| /// Sets the candidates attribute of every sol.icall. | ||
| void annotateICallCandidates(ModuleOp mod) { | ||
| Builder b(mod.getContext()); | ||
| DenseMap<std::pair<Operation *, sol::FuncRefType>, SetVector<int64_t>> | ||
| buckets; | ||
| SmallVector<sol::ICallOp> icalls; | ||
| mod.walk([&](Operation *op) { | ||
| if (auto icall = dyn_cast<sol::ICallOp>(op)) { | ||
| icalls.push_back(icall); | ||
| return; | ||
| } | ||
| auto fnConst = dyn_cast<sol::FuncConstantOp>(op); | ||
| if (!fnConst) | ||
| return; | ||
| auto fn = SymbolTable::lookupNearestSymbolFrom<sol::FuncOp>( | ||
| fnConst, fnConst.getSymAttr()); | ||
| assert(fn && fn.getId() && "unresolvable sol.func_constant"); | ||
| if (!fn || !fn.getId()) | ||
| return; | ||
|
Comment on lines
+315
to
+317
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The assert and the consequent |
||
| buckets[{fnConst->getParentOfType<ModuleOp>().getOperation(), | ||
| cast<sol::FuncRefType>(fnConst.getType())}] | ||
| .insert(*fn.getId()); | ||
| }); | ||
| for (sol::ICallOp icall : icalls) { | ||
| SmallVector<int64_t> ids; | ||
| auto it = | ||
| buckets.find({icall->getParentOfType<ModuleOp>().getOperation(), | ||
| cast<sol::FuncRefType>(icall.getCallee().getType())}); | ||
| if (it != buckets.end()) | ||
| ids.assign(it->second.begin(), it->second.end()); | ||
| llvm::sort(ids); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suggest to move sorting from this loop to do sort only once, finalize sorted attribute creation and then use them to annotate icalls. We could do something like: |
||
| icall.setCandidatesAttr(b.getDenseI64ArrayAttr(ids)); | ||
| } | ||
| } | ||
|
|
||
| void runOnOperation() override { | ||
| ModuleOp mod = getOperation(); | ||
| evm::SolTypeConverter tyConv; | ||
| annotateICallCandidates(mod); | ||
| if (failed(runStage1Conversion(mod, tyConv))) { | ||
| signalPassFailure(); | ||
| return; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest to add an assert that the enclosing function's ancestor is already a yul.object.