[Moore] [3/4] Implement class member access, upcast #9135
Conversation
Scheremo
force-pushed
the
pr-moore-class-memberaccess
branch
20 times, most recently
from
008177b to
7ea2c46
Compare
fabianschuiki
left a comment
fabianschuiki
left a comment
There was a problem hiding this comment.
LGTM, modulo the O(N²) behavior thanks to MLIR's SymbolTables.
| let arguments = (ins ClassHandleType:$instance); | ||
| let results = (outs ClassHandleType:$result); | ||
| let assemblyFormat = | ||
| "$instance `:` type($instance) `to` type($result) attr-dict"; |
There was a problem hiding this comment.
I'm wondering if it would make sense to constrain the upcast op to only perform a single layer of upcasting; basically a class to one of its immediate base classes. This would potentially simplify lowering of the op, since you can just perform a single projection on the class pointer and the vtable pointer. The frontend could then insert multiple upcast ops if needed. WDYT?
There was a problem hiding this comment.
So my original thought was to have "multi-step" upcasts for a few reasons:
- I think it looks neater 😄
- System Verilog only allows single-inheritance from base classes, (but multiple "inheritance" for interfaces - however all of their fields must be virtual and there is no shadowing, so vtable resolution shouldn't produce the diamond inheritance problem) so the upcast path is unique, there's no diamond inheritance problem.
- My idea was to translate upcasts into literal no-ops; we practically only need the instance's RTTI to decide on the vtable pointer for dispatch, and since we can rely on objects only having single inheritance, the class pointer upcast is straight-forward. We can just construct the object layout to append properties to its data structure for each level of inheritance.
Maybe I'm missing something but I believe multi-step upcasting is safe. I think your solution works equally well, but since we don't have multiple inheritance we should be able to skip it; thoughts?
There was a problem hiding this comment.
Makes sense! I think the approach you have in this PR is great!
Scheremo
force-pushed
the
pr-moore-class-memberaccess
branch
from
3a0ef15 to
a2aedcc
Compare
- **New IR ops** - `moore.class.property_ref`: yields a `!moore.ref<T>` to a class property. - `moore.class.upcast`: converts a derived class handle to one of its base class handles (zero-cost). - **Class handle utilities** - Added `Context::isSameOrDerivedFrom()` and `getAncestorClassWithProperty()` for inheritance and field lookup. - Added verifiers for both new ops ensuring valid symbols & types. - `ImportVerilog/Expressions.cpp`: - Added implicit handle upcasting (`maybeUpcastHandle`). - Added lowering for `t.a` reads/writes via `moore.class.property_ref`. - Emits `moore.class.upcast` when accessing inherited fields. - Added coverage for: - Property read/write (`t.a`, `t.a = x`). - Implicit upcast from derived -> base before member access.
Scheremo
force-pushed
the
pr-moore-class-memberaccess
branch
from
a2aedcc to
4cb203b
Compare
2 participants
New IR ops
moore.class.property_ref: yields a!moore.ref<T>to a class property.moore.class.upcast: converts a derived class handle to one of its base class handles (zero-cost).Class handle utilities
Context::isSameOrDerivedFrom()andgetAncestorClassWithProperty()for inheritance and field lookup.ImportVerilog/Expressions.cpp:maybeUpcastHandle).t.areads/writes viamoore.class.property_ref.moore.class.upcastwhen accessing inherited fields.Added coverage for:
t.a,t.a = x).