Skip to content
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

Make IRWitnessTable HOISTABLE #6417

Open
wants to merge 21 commits into
base: master
Choose a base branch
from
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Update the comments
jkwak-work committed Mar 20, 2025
commit 8189082b6b25207fbb7c825cfa3a63d27cf2539d
11 changes: 8 additions & 3 deletions source/slang/slang-ir-specialize.cpp
Original file line number Diff line number Diff line change
@@ -3123,13 +3123,18 @@ IRInst* specializeGenericImpl(
//
IRInstList<IRInst> ordinaryInsts = bb->getOrdinaryInsts();

// After IRWitnessTable became Hoistable, they are removed and insered back by
// After IRWitnessTable became Hoistable, they are removed and inserted back by
// `addHoistableInst()`. But when they are re-inserted, the order it appears in the block is
// changed. When IRWitnessTable refers to IRSpecialize, as an example, IRSpecialize must be
// changed. We need to change the order in a way that the dependancy is resolved.
//
// The dependency cannot be resolve in `addHoistableInst()`, because IRWitnessTable doesn't
// have IRWitnessTableEntry yet while in the function.
//
// When IRWitnessTable refers to IRSpecialize, as an example, IRSpecialize must be
// cloned before the cloning of IRWitnessTable. It is because the operands are assumed to be
// cloned before the cloning of IRInst.
//
// We need to resolve the dependency problem by changing the order of them.
// Similarly, there can be dependencies between an IRWitnessTable and another IRWitnessTable.
//
List<IRInst*> insts;
int instCount = 0;
18 changes: 6 additions & 12 deletions source/slang/slang-lower-to-ir.cpp
Original file line number Diff line number Diff line change
@@ -8046,12 +8046,9 @@ struct DeclLoweringVisitor : DeclVisitor<DeclLoweringVisitor, LoweredValInfo>
irWitnessTableBaseType,
irWitnessTable->getConcreteType());

// TODO: When WitnessTable became HOISTABLE, we needed a way to avoid
// adding the same decorations and childrens redundantly.
// There isn't an easy way to tell if the returned WitnessTable is a
// brand new or a found one from the existing pool.
// The code below assumes that when there are any decoration or child,
// it is a pre-existed one.
// Since IRWitnessTable is Hoistable, `createWitnessTable()` may return a
// pre-existing one. We need to avoid adding the same decorations/children
// when the IRWitnessTable already has them.
//
if (irSatisfyingWitnessTable->getFirstDecorationOrChild() == nullptr)
{
@@ -8197,12 +8194,9 @@ struct DeclLoweringVisitor : DeclVisitor<DeclLoweringVisitor, LoweredValInfo>
// Create the IR-level witness table
auto irWitnessTable = subBuilder->createWitnessTable(irWitnessTableBaseType, irSubType);

// TODO: When WitnessTable became HOISTABLE, we needed a way to avoid
// adding the same decorations and childrens redundantly.
// There isn't an easy way to tell if the returned WitnessTable is a
// brand new or a found one from the existing pool.
// The code below assumes that when there are any decoration or child,
// it is a pre-existed one.
// Since IRWitnessTable is Hoistable, `createWitnessTable()` may return a
// pre-existing one. We need to avoid adding the same decorations/children
// when the IRWitnessTable already has them.
//
if (irWitnessTable->getFirstDecorationOrChild() == nullptr)
{