[SandboxIR][NFC] Introduce templated CastInstImpl to simplify subclasses #101427
Conversation
The CastInst subclasses all have pretty much the same implementation. Add a helper templated class to help stamp out the subclasses more succinctly.
| class SIToFPInst final : public CastInstImpl<Instruction::Opcode::SIToFP> {}; | ||
| class FPToUIInst final : public CastInstImpl<Instruction::Opcode::FPToUI> {}; | ||
| class FPToSIInst final : public CastInstImpl<Instruction::Opcode::FPToSI> {}; | ||
| class IntToPtrInst final : public CastInstImpl<Instruction::Opcode::IntToPtr> { |
There was a problem hiding this comment.
clang-format shrug
aeubanks
requested review from
vporpo,
tmsri,
tschuett,
slackito,
Sterling-Augustine and
alinas
| } | ||
| #ifndef NDEBUG |
There was a problem hiding this comment.
I would keep these for now until we find a better fix for all of them. The issue is that gdb won't work with I->dump() if I is a pointer to a sub-class of CastInst. You can get it to work by casting it to sandboxir::CastInst *, like (gdb) ((sandboxir::CastInst *)I)->dump() which is annoying.
There was a problem hiding this comment.
it looks like all dump() methods are the same right now, with multi-instructions being different in the future. there's gotta be a better way of deduplicating some of the logic, seeing so many copies of
void FooInst::dump(raw_ostream &OS) const {
dumpCommonPrefix(OS);
dumpCommonSuffix(OS);
}
hurts.
can you test if B().f() in
struct A {
void f();
};
struct B : A {
using A::f;
};
works with gdb?
There was a problem hiding this comment.
Yeah there is a ton of duplication. What might work is a non-virtual Value::dump() that calls virtual dumpImpl() that may get overriden by the sub-classes if needed.
I added a using CastInst::dump in SIToFPInst, but gdb still complains: Couldn't find method llvm::sandboxir::SIToFPInst::dump.
The CastInst subclasses all have pretty much the same implementation. Add a helper templated class to help stamp out the subclasses more succinctly.