Fix an assert violation and a SIGSEGV in emap

[marcelwa]

This PR fixes an assert violation and a SIGSEGV in the emap technology mapper. The former resulted from a missing value reset and a floating-point type mismatch, and the latter stemmed from an overlooked corner case in arrival time propagation.

The two assert statements (lines 1666-1667)

assert( node_match[index].arrival[0] < node_match[index].required[0] + epsilon );
assert( node_match[index].arrival[1] < node_match[index].required[1] + epsilon );

could be triggered because the arrival and required times of node_data and node_match were not always correctly re-initialized.

Furthermore (line 2364),

double arrival_pin = node_match[l].arrival[( best_phase >> ctr ) & 1] + best_gate->tdelay[ctr];

could cause a SIGSEGV when neither phase has a best gate. These cases must be skipped.

The proposed changes fix these issues.

[marcelwa]

I see that my changes cause emap's quality of result to differ from the specified values in the test cases, which was to be expected. In some cases, it seems to perform better; in others, it performs worse. It would be great to hear your thoughts on this and on how to proceed.

[aletempiac]

Hi Marcel,

Thank you for reporting this, could you share the stack trace and how to reproduce?
I am not sure this is correct way to fix the bug.

One thing that I notice is that in method set_mapping_refs_and_req

set_output_required_time( iteration == 0 );

should be

if ( !ps.area_oriented_mapping )
{
    set_output_required_time( iteration == 0 );
}

Let me know if this alone fixes the bug.

Thanks,
Alessandro

[marcelwa]

Hi Alessandro,

Many thanks for the quick feedback. I'll run your fix through my test cases and will report back on how it went. I'll see if I can even condense them into a test case for mockturtle to reproduce the issue first-hand.

Thanks again!

[marcelwa]

@aletempiac, as promised, I added a failing test case that triggers the assertions (and throws a SIGSEGV). Unfortunately, your proposed fix doesn't prevent the issue, whereas the changes I made in 32ba90e do. Hence, to reproduce the error, you would have to roll back those lines.

I want to emphasize that my changes could very well cause other unintended behavior that I'm simply not aware of, as I do not know the code as well as you do. I'm looking forward to hearing what you think and how to best proceed.

[aletempiac]

Hi Marcel,

I can see that one problem is related to the library not defining the XOR cell. If you use an XAG as a representation {AND2, XOR2, and INV} must be present in the library. Else, I suggest using an AIG. This would solve the problem.

Alternatively, you might add an if statement around the assertions testing that node_match[index].best_gate[0] != nullptr || node_match[index].best_gate[1] != nullptr, i.e., a valid mapping exists, but you might still face problems in other parts of the code.

Best,
Alessandro

[marcelwa]

Thanks for the insight, Alessandro! Is there any way to test whether the library is complete for the given initial network type prior to performing the tech mapping? That would enable a smoother integration into user-facing code because it could fail gracefully with meaningful error messages.

[aletempiac]

What if you check that each primitive is available in the library up to output negation (e.g, AIG: AND2 and INV; XAG: AND2, XOR2, INV). You can add a check over the genlib library matching by truth table.

[marcelwa]

Thank you, Alessandro. That's how I currently handle it as well. I was kinda hoping that there was a more elegant solution. But if you'd recommend this path instead of code adjustments to emap, I understand. We can then close this PR.