GPUOpen-Drivers / llvm-dialects

LLVM Dialects Library
Apache License 2.0
22 stars 22 forks source link

Implement support for pre-visitor callbacks. #84

Open tsymalla opened 8 months ago

tsymalla commented 8 months ago

This change implements a mechanism to run pre-visit callbacks on each operation it stumbles upon. This can be generalized for every op that is being visited, for the current nesting level, or for a given OpSet.

The code simplifies writing visitors in cases where generic code is written for a set operations, like resetting the insertion point of an IRBuilder.

Note: the support is currently basic. When adding a pre-visitor callback for a given nesting level without an operation, this will operate for each instruction types with visitor callbacks. I'd like to extend this so we only operate on the set of operations registered for the current visitor nesting level or its parents, but I'm afraid we don't support that yet. In addition, I plan on cleaning up all of the forwarding code and surroundings next, since it became a bit of a mess with all of the repetetive code.

@nhaehnle

nhaehnle commented 8 months ago

Or perhaps it shouldn't be a "pre-visitor" but a sort of scope guard:

  .withGuard(&Blah::someGuard, [](VisitorBuilder<Blah> &b) { ... })

...

VisitorResult Blah::someGuard(Instruction &inst, llvm::function_ref<VisitorResult()> visit) {
  if (should skip)
    return VisitorResult::Continue;

  do some setup
  auto result = visit(); // this calls the visitors that were registered inside of the "withGuard" block
  do some teardown

  return result;
}
tsymalla-AMD commented 8 months ago

Or perhaps it shouldn't be a "pre-visitor" but a sort of scope guard:

  .withGuard(&Blah::someGuard, [](VisitorBuilder<Blah> &b) { ... })

...

VisitorResult Blah::someGuard(Instruction &inst, llvm::function_ref<VisitorResult()> visit) {
  if (should skip)
    return VisitorResult::Continue;

  do some setup
  auto result = visit(); // this calls the visitors that were registered inside of the "withGuard" block
  do some teardown

  return result;
}

Having separate setup and teardown callbacks per-op (or for all ops) as part of the guards sounds like a good way, and we'd nest all visitor callbacks in a top-level guard with a setup callback to do something like setting the Builder insertion point.