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

Verify tmp registers #1010

Open
wants to merge 24 commits into
base: main
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
10 changes: 10 additions & 0 deletions compiler/modules/front/stmt.bal
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,10 @@ class StmtContext {
return bir:createTmpRegister(self.code, t, pos);
}

function createAssignTmpRegister(bir:SemType t, Position? pos = ()) returns bir:AssignTmpRegister {
return bir:createAssignTmpRegister(self.code, t, pos);
}

function nextRegisterNumber() returns int {
return self.code.registers.length();
}
Expand Down Expand Up @@ -699,6 +703,12 @@ function codeGenMatchStmt(StmtContext cx, bir:BasicBlock startBlock, Environment
else {
// Match expression is not a variable: we do not get type narrowing
int patternIndex = 0;
if matched is bir:TmpRegister {
bir:AssignTmpRegister result = cx.createAssignTmpRegister(matched.semType, matched.pos);
bir:AssignInsn insn = { result, operand: matched, pos : stmt.startPos };
matched = result;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In a match we are only reading matched, aren't we ? why do we need AssignTmpRegister?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In a case like

match x + 1 {
    2 => { 
        return "1";
    }
    4 => {
        return "2";
    }
}

even though matched is a TmpRegister it will be used in multiple blocks for Equality instructions, so to avoid that I think we can either do something like this or change all expr code gens to create AssignTmpRegister instead of Tmp depending on the situation.

Copy link
Contributor

@manuranga manuranga May 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we'll have to change the definition of TmpRegister to, assigned once, but can be used form multiple blocks.
@jclark what do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this is already the case, the problem is when the TmpRegister is used in a different block than the block it was initially assigned.

Copy link
Contributor

@manuranga manuranga May 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but can be used multiple times

I meant

but can be used from multiple block

let me edit.

testBlock.insns.push(insn);
}
foreach var mt in matchTests {
int clauseIndex = mt.clauseIndex;
if clauseIndex == defaultClauseIndex {
Expand Down