Skip to content

Global SEC check#4329

Open
nanocoh wants to merge 28 commits into
The-OpenROAD-Project:masterfrom
keplertech:global-sec
Open

Global SEC check#4329
nanocoh wants to merge 28 commits into
The-OpenROAD-Project:masterfrom
keplertech:global-sec

Conversation

@nanocoh

@nanocoh nanocoh commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

No description provided.

Noam Cohen and others added 26 commits May 12, 2026 01:17
Signed-off-by: Noam Cohen <noam.chn1@gmail.com>
Signed-off-by: Noam Cohen <noam.chn1@gmail.com>
Signed-off-by: Noam Cohen <noam.chn1@gmail.com>
Signed-off-by: Noam Cohen <noam.chn1@gmail.com>
Signed-off-by: Noam Cohen <noam.chn1@gmail.com>
Signed-off-by: Noam Cohen <noam.chn1@gmail.com>
Signed-off-by: Noam Cohen <noam.chn1@gmail.com>
Signed-off-by: Noam Cohen <noam.chn1@gmail.com>
Signed-off-by: Noam Cohen <noam.chn1@gmail.com>
Signed-off-by: Noam Cohen <noam.chn1@gmail.com>
Signed-off-by: Noam Cohen <noam.chn1@gmail.com>
Signed-off-by: Noam Cohen <noam.chn1@gmail.com>
Signed-off-by: Noam Cohen <noam.chn1@gmail.com>
Signed-off-by: Noam Cohen <noam.chn1@gmail.com>
Signed-off-by: Noam Cohen <noam.chn1@gmail.com>
Signed-off-by: Noam Cohen <noam.chn1@gmail.com>
Signed-off-by: Noam Cohen <noam.chn1@gmail.com>
Signed-off-by: Noam Cohen <noam.chn1@gmail.com>
Signed-off-by: Noam Cohen <noam.chn1@gmail.com>
Signed-off-by: Noam Cohen <noam.chn1@gmail.com>
Signed-off-by: Noam Cohen <noam.chn1@gmail.com>
Signed-off-by: Noam Cohen <noam.chn1@gmail.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request integrates sequential equivalence checking (SEC_CHECK) using the Kepler Formal PDR engine into the flow, alongside existing logical equivalence checks (LEC_CHECK). The changes update documentation, variables schemas, Makefiles, and synthesis/reporting scripts to support these checks, as well as updating the kepler-formal submodule. The code review feedback is highly constructive and should be addressed: it recommends adding helper functions (lec_check_enabled and sec_check_enabled) to safely guard execution rather than directly reading environment variables, verifying the existence of the SEC log file before running grep to avoid silent failures, and dynamically resolving step labels in run_sec_test for consistency.

Comment on lines +90 to 105
proc run_sec_test { step file1 file2 } {
write_sec_script $step $file1 $file2
# tclint-disable-next-line command-args
eval exec $::env(KEPLER_FORMAL_EXE) --config $::env(OBJECTS_DIR)/${step}_sec_test.yml
try {
set count [exec grep -c "SEC found a counterexample" $::env(LOG_DIR)/${step}_sec_check.log]
} trap CHILDSTATUS {results options} {
# This block executes if grep returns a non-zero exit code
set count 0
}
if { $count > 0 } {
error "Repair timing output failed lec test"
error "Global output failed sec test"
} else {
puts "Repair timing output passed lec test"
puts "Global output passed sec test"
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

Checking for a pattern in the log file using grep without first verifying that the log file exists can lead to silent passes if the formal tool fails to run or crashes (since grep exiting with status 2 on a missing file will trigger the CHILDSTATUS trap and set count to 0). Additionally, using a hardcoded label like "Global output" is inconsistent with run_lec_test which uses formal_check_label to dynamically resolve the step label.

proc run_sec_test { step file1 file2 } {
  write_sec_script $step $file1 $file2
  # tclint-disable-next-line command-args
  eval exec $::env(KEPLER_FORMAL_EXE) --config $::env(OBJECTS_DIR)/${step}_sec_test.yml
  set log_file $::env(LOG_DIR)/${step}_sec_check.log
  if { ![file exists $log_file] } {
    error "SEC log file not found: $log_file"
  }
  try {
    set count [exec grep -c "SEC found a counterexample" $log_file]
  } trap CHILDSTATUS {results options} {
    # This block executes if grep returns a non-zero exit code
    set count 0
  }
  set label [formal_check_label $step]
  if { $count > 0 } {
    error "$label failed sec test"
  } else {
    puts "$label passed sec test"
  }
}

Comment on lines +23 to +33
if { $::env(LEC_CHECK) || $::env(SEC_CHECK) } {
write_lec_verilog 6_final_lec.v
}

if { $::env(LEC_CHECK) } {
run_lec_test 6_final 1_synth_lec.v 6_final_lec.v
}

if { $::env(SEC_CHECK) } {
run_sec_test 6_final 1_synth_lec.v 6_final_lec.v
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

Using raw environment variables like $::env(LEC_CHECK) and $::env(SEC_CHECK) directly to guard formal check execution can cause crashes if they are set to 1 but the formal tool executable is missing or not executable. Use the lec_check_enabled and sec_check_enabled helper functions to safely guard these steps.

if { [lec_check_enabled] || [sec_check_enabled] } {
  write_lec_verilog 6_final_lec.v
}

if { [lec_check_enabled] } {
  run_lec_test 6_final 1_synth_lec.v 6_final_lec.v
}

if { [sec_check_enabled] } {
  run_sec_test 6_final 1_synth_lec.v 6_final_lec.v
}

Comment on lines +65 to +70
proc formal_check_label { step } {
if { [string equal $step 4_rsz] } {
return "Repair timing output"
}
return "Global output"
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Add a sec_check_enabled helper function to check if sequential equivalence checking is enabled and the Kepler Formal executable is available and executable, matching the design of lec_check_enabled.

proc formal_check_label { step } {
  if { [string equal $step 4_rsz] } {
    return "Repair timing output"
  }
  return "Global output"
}

proc sec_check_enabled { } {
  return [expr {
    [env_var_equals SEC_CHECK 1]
    && [info exists ::env(KEPLER_FORMAL_EXE)]
    && [file executable $::env(KEPLER_FORMAL_EXE)]
  }]
}

Comment on lines +38 to +40
if { $::env(LEC_CHECK) || $::env(SEC_CHECK) } {
write_lec_verilog 1_synth_lec.v
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Use lec_check_enabled and sec_check_enabled helper functions to guard writing the LEC verilog file, ensuring consistency and preventing unnecessary file generation when the formal check tool is not available.

if { [lec_check_enabled] || [sec_check_enabled] } {
  write_lec_verilog 1_synth_lec.v
}

Comment on lines +78 to +80
if { $::env(LEC_CHECK) || $::env(SEC_CHECK) } {
write_lec_verilog 1_synth_lec.v
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Use lec_check_enabled and sec_check_enabled helper functions to guard writing the LEC verilog file, ensuring consistency and preventing unnecessary file generation when the formal check tool is not available.

if { [lec_check_enabled] || [sec_check_enabled] } {
  write_lec_verilog 1_synth_lec.v
}

Noam Cohen added 2 commits July 3, 2026 14:37
Signed-off-by: Noam Cohen <noam.chn1@gmail.com>
Signed-off-by: Noam Cohen <noam.chn1@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant