Skip to content

Commit bb0e6f0

Browse files
Copilotn0thingNoob
andcommitted
refactor: improve PHI validation and extract test helper
- Replace silent fallback in PHI with explicit panic when no predecessor matches - Extract duplicate ProgramIR building logic into buildProgramIR helper function - Fix misleading comment about PC value after JMP instruction Co-authored-by: n0thingNoob <37942719+n0thingNoob@users.noreply.github.com>
1 parent 238b624 commit bb0e6f0

2 files changed

Lines changed: 47 additions & 81 deletions

File tree

core/emu_ir.go

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -68,15 +68,9 @@ func (i instEmulator) runPhiIR(inst Instruction, state *coreState) {
6868
}
6969

7070
if !foundMatch {
71-
// If no match found, this might be the first execution or an error
72-
// For safety, we could use the first value or panic
73-
// Let's use first value for robustness
74-
firstIncoming := strings.TrimSpace(inst.Operands[1])
75-
parts := strings.Split(firstIncoming, "@")
76-
if len(parts) >= 1 {
77-
srcReg := strings.TrimSpace(parts[0])
78-
selectedValue = i.readOperand(srcReg, state)
79-
}
71+
// No matching predecessor found for PHI; this indicates a control flow or PHI construction error.
72+
panic(fmt.Sprintf("PHI: no incoming value matches predecessor block '%s' at PC %d; operands: %v",
73+
predBlock, state.PC, inst.Operands))
8074
}
8175

8276
// Write the selected value to destination

core/emu_unit_test.go

Lines changed: 44 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,45 @@ var _ = Describe("InstEmulator", func() {
229229
})
230230
})
231231

232+
// Helper function to build ProgramIR from program strings
233+
buildProgramIR := func(program []string, state *coreState) {
234+
state.Code = program
235+
state.ProgramIR = make([]Instruction, len(program))
236+
state.PCToBlock = make(map[uint32]string)
237+
238+
currentBlock := ""
239+
for i, line := range program {
240+
line = strings.TrimSpace(line)
241+
if strings.HasSuffix(line, ":") {
242+
labelName := strings.TrimSuffix(line, ":")
243+
labelName = strings.TrimSpace(labelName)
244+
state.ProgramIR[i] = Instruction{
245+
Label: labelName,
246+
Raw: line,
247+
}
248+
currentBlock = labelName
249+
} else {
250+
tokens := strings.Split(line, ",")
251+
opcode := ""
252+
operands := []string{}
253+
if len(tokens) > 0 {
254+
opcode = strings.TrimSpace(tokens[0])
255+
for j := 1; j < len(tokens); j++ {
256+
operands = append(operands, strings.TrimSpace(tokens[j]))
257+
}
258+
}
259+
state.ProgramIR[i] = Instruction{
260+
Opcode: Opcode(opcode),
261+
Operands: operands,
262+
Raw: line,
263+
}
264+
if currentBlock != "" {
265+
state.PCToBlock[uint32(i)] = currentBlock
266+
}
267+
}
268+
}
269+
}
270+
232271
Context("PHI Instruction with IR", func() {
233272
Describe("PHI based on predecessor block", func() {
234273
It("should select value from correct predecessor block", func() {
@@ -253,42 +292,8 @@ var _ = Describe("InstEmulator", func() {
253292
"DONE",
254293
}
255294

256-
// Initialize ProgramIR and PCToBlock
257-
s.Code = program
258-
s.ProgramIR = make([]Instruction, len(program))
259-
s.PCToBlock = make(map[uint32]string)
260-
261-
currentBlock := ""
262-
for i, line := range program {
263-
line = strings.TrimSpace(line)
264-
if strings.HasSuffix(line, ":") {
265-
labelName := strings.TrimSuffix(line, ":")
266-
labelName = strings.TrimSpace(labelName)
267-
s.ProgramIR[i] = Instruction{
268-
Label: labelName,
269-
Raw: line,
270-
}
271-
currentBlock = labelName
272-
} else {
273-
tokens := strings.Split(line, ",")
274-
opcode := ""
275-
operands := []string{}
276-
if len(tokens) > 0 {
277-
opcode = strings.TrimSpace(tokens[0])
278-
for j := 1; j < len(tokens); j++ {
279-
operands = append(operands, strings.TrimSpace(tokens[j]))
280-
}
281-
}
282-
s.ProgramIR[i] = Instruction{
283-
Opcode: Opcode(opcode),
284-
Operands: operands,
285-
Raw: line,
286-
}
287-
if currentBlock != "" {
288-
s.PCToBlock[uint32(i)] = currentBlock
289-
}
290-
}
291-
}
295+
// Initialize ProgramIR and PCToBlock using helper
296+
buildProgramIR(program, &s)
292297

293298
// Simulate coming from block A
294299
s.PC = 0
@@ -308,7 +313,7 @@ var _ = Describe("InstEmulator", func() {
308313

309314
// Execute JMP B (PC=2) - this sets PC to B's label
310315
ie.RunInst("JMP, B", &s)
311-
Expect(s.PC).To(Equal(uint32(7))) // PC should jump to "B:" label + 1
316+
Expect(s.PC).To(Equal(uint32(7))) // PC should jump to B block (where PHI is located)
312317

313318
// Now we're entering block B from A
314319
// Update LastPredBlock before executing PHI
@@ -339,41 +344,8 @@ var _ = Describe("InstEmulator", func() {
339344
"DONE",
340345
}
341346

342-
s.Code = program
343-
s.ProgramIR = make([]Instruction, len(program))
344-
s.PCToBlock = make(map[uint32]string)
345-
346-
currentBlock := ""
347-
for i, line := range program {
348-
line = strings.TrimSpace(line)
349-
if strings.HasSuffix(line, ":") {
350-
labelName := strings.TrimSuffix(line, ":")
351-
labelName = strings.TrimSpace(labelName)
352-
s.ProgramIR[i] = Instruction{
353-
Label: labelName,
354-
Raw: line,
355-
}
356-
currentBlock = labelName
357-
} else {
358-
tokens := strings.Split(line, ",")
359-
opcode := ""
360-
operands := []string{}
361-
if len(tokens) > 0 {
362-
opcode = strings.TrimSpace(tokens[0])
363-
for j := 1; j < len(tokens); j++ {
364-
operands = append(operands, strings.TrimSpace(tokens[j]))
365-
}
366-
}
367-
s.ProgramIR[i] = Instruction{
368-
Opcode: Opcode(opcode),
369-
Operands: operands,
370-
Raw: line,
371-
}
372-
if currentBlock != "" {
373-
s.PCToBlock[uint32(i)] = currentBlock
374-
}
375-
}
376-
}
347+
// Initialize ProgramIR and PCToBlock using helper
348+
buildProgramIR(program, &s)
377349

378350
// Simulate coming from block C
379351
s.PC = 3 // Start at C:

0 commit comments

Comments
 (0)