Adding LRES table for GF180MCU DRC #65

FaragElsayed2 · 2023-04-03T15:17:42Z

Adding LRES table for GF180MCU DRC

Fixes #43

atorkmabrains · 2023-04-03T22:54:05Z

@proppy Looking forward to getting your feedback on all the PRs.

atorkmabrains · 2023-04-04T18:34:09Z

@proppy ?

proppy · 2023-04-04T22:51:43Z

@atorkmabrains can you review it first?

atorkmabrains · 2023-04-04T23:11:16Z

@proppy I have already reviewed and approved that code before. My concerns are different than yours. I always focus on 2 things:

Correctness of DRC
Performance of code against large designs.

All the code that you have here is already reviewed and approved multiple weeks before.

proppy · 2023-04-04T23:13:08Z

@atorkmabrains are you able to mark the PR as approved here?

atorkmabrains

Thanks @FaragElsayed2

atorkmabrains · 2023-04-04T23:14:42Z

@proppy Done

proppy · 2023-04-05T04:29:01Z

klayout/drc/rule_decks/lres.drc

+# limitations under the License.
+################################################################################################
+
+if FEOL


#58 but not blocking

proppy · 2023-04-05T04:30:38Z

klayout/drc/rule_decks/lres.drc

+
+if FEOL
+  #================================================
+  #----------------N+ POLY RESISTOR----------------


#57 but not blocking

proppy · 2023-04-05T04:30:50Z

klayout/drc/rule_decks/lres.drc

+
+if FEOL
+  #================================================
+  #----------------N+ POLY RESISTOR----------------


#57 but not blocking

proppy · 2023-04-05T04:31:20Z

klayout/drc/rule_decks/lres.drc

+  logger.info('Executing rule LRES.1')
+  lres1_l1 = lres_poly.width(0.8.um, euclidian)
+  lres1_l1.output('LRES.1', 'LRES.1 : Minimum width of Poly2 resistor. : 0.8µm')
+  lres1_l1.forget


#53 but not blocking

proppy · 2023-04-05T04:31:25Z

klayout/drc/rule_decks/lres.drc

+  logger.info('Executing rule LRES.1')
+  lres1_l1 = lres_poly.width(0.8.um, euclidian)
+  lres1_l1.output('LRES.1', 'LRES.1 : Minimum width of Poly2 resistor. : 0.8µm')
+  lres1_l1.forget


#53 but not blocking

proppy · 2023-04-05T04:31:28Z

klayout/drc/rule_decks/lres.drc

+  logger.info('Executing rule LRES.1')
+  lres1_l1 = lres_poly.width(0.8.um, euclidian)
+  lres1_l1.output('LRES.1', 'LRES.1 : Minimum width of Poly2 resistor. : 0.8µm')
+  lres1_l1.forget


#53 but not blocking

proppy · 2023-04-05T04:32:28Z

klayout/drc/rule_decks/lres.drc

+  ## and both side (X and Y) are greater than 80um.
+  ## then the minimum spacing to adjacent RES_MK layer. is 20µm
+  logger.info('Executing rule LRES.9b')
+  lres9b = res_mk.with_area(15_000.001.um, nil).edges.with_length(80.001.um, nil)


why do we have .001 here?

As the rule wording explains, greater than 15000 not greater than or equal.

what about using 15_000.um + 1.dbu instead? that seems less arbitrary than .001?

filed #67 to discuss this further

proppy · 2023-04-05T04:34:11Z

klayout/drc/rule_decks/lres.drc

+
+  # Rule LRES.3: Minimum space from Poly2 resistor to COMP.
+  logger.info('Executing rule LRES.3')
+  lres3_l1 = lres_poly.separation(comp, 0.6.um, euclidian).polygons(0.001.um).or(comp.not_outside(lres_poly))


why do we have .001 here?

All polygons(0.001.um) are for visibility and being able to join the results. But we can't remove that as we combine the layers down the line as well.

can you clarify what "All polygons(0.001.um) are for visibility" means? (or even better add a comment in the rule that explain that)

When run any kind of spacing check, it generates edges rather than polygons. We convert them to polygons with with 0.001.um. This is what this does.

and what is the reason we don't keep them as edges? is to be able to do or() on them or to be able to call output()?

@proppy exactly.

would https://www.klayout.de/doc/code/class_EdgeProcessor.html#method43 also work here? what would be the trade off of using one versus the other?

We haven't used that method. Also, we don't know if it's exposed for usage in DRC or not. Keep in mind that some of those might not be exposed to be used in DRC. polygons has been tested significantly. It's fast and tested.

@proppy I don't mind that you could test out the function that you have highlighted above. If it's faster or better, happy to change. But if there is no impact, I recommend leaving polygons as is.

filed #67 to discuss this further

proppy · 2023-04-05T04:34:17Z

klayout/drc/rule_decks/lres.drc

+
+  # Rule LRES.5: Minimum Nplus implant overlap of Poly2 resistor. is 0.3µm
+  logger.info('Executing rule LRES.5')
+  lres5_l1 = lres_poly.enclosed(nplus, 0.3.um, euclidian).polygons(0.001.um)


why do we have .001 here?

filed #66 to discuss this further

proppy · 2023-04-05T04:34:25Z

klayout/drc/rule_decks/lres.drc

+  # Rule LRES.7: Space from salicide block to contact on Poly2 resistor.
+  logger.info('Executing rule LRES.7')
+  cont_lres7 = contact.and(lres_poly)
+  lres7_l1 = cont_lres7.separation(sab, 0.22.um).polygons(0.001.um)


why do we have .001 here?

filed #66 to discuss this further

klayout/drc/rule_decks/lres.drc

proppy · 2023-04-05T04:39:10Z

klayout/drc/rule_decks/lres.drc

+  lres7_l2.forget
+  cont_lres7.forget
+
+  # Rule LRES.8 is not a DRC check


#55 but not blocking

proppy

just some nits and questions but nothing blocking per se, I'll leave it up to you if you want to address those in this PR or file separate issues to track them.

atorkmabrains · 2023-04-05T13:28:38Z

@proppy Are you using some form of automation to send those comments? I believe it seems that you are doing so. If yes, could you please enlighten me how you do that?

Also, are you doing some form of code scanners?

klayout/drc/rule_decks/lres.drc

proppy · 2023-04-06T04:42:11Z

@proppy Are you using some form of automation to send those comments? I believe it seems that you are doing so. If yes, could you please enlighten me how you do that?

No, just way ETOOMANY many tabs open :)

Also, are you doing some form of code scanners?

My eyes and grep?

Adding LRES table for GF180MCU DRC

e80c05c

mithro assigned proppy and unassigned proppy Apr 3, 2023

mithro requested a review from proppy April 3, 2023 17:31

This was referenced Apr 4, 2023

Adding PRES table for GF180MCU DRC #64

Merged

Adding HRES table for GF180MCU DRC #63

Merged

Adding SRAM5p0 table for GF180MCU DRC #62

Merged

Adding MCELL table for GF180MCU DRC #60

Merged

atorkmabrains mentioned this pull request Apr 4, 2023

Adding SRAM3p3 table for GF180MCU DRC #61

Merged

atorkmabrains self-requested a review April 4, 2023 23:13

atorkmabrains approved these changes Apr 4, 2023

View reviewed changes

proppy reviewed Apr 5, 2023

View reviewed changes

klayout/drc/rule_decks/lres.drc Outdated Show resolved Hide resolved

proppy reviewed Apr 5, 2023

View reviewed changes

klayout/drc/rule_decks/lres.drc Outdated Show resolved Hide resolved

proppy reviewed Apr 5, 2023

View reviewed changes

klayout/drc/rule_decks/lres.drc

lres7_l2.forget

cont_lres7.forget

# Rule LRES.8 is not a DRC check

Copy link

Member

proppy Apr 5, 2023

Choose a reason for hiding this comment

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

#55 but not blocking

proppy approved these changes Apr 5, 2023

View reviewed changes

Updating comments for DRC rules

668b144

FaragElsayed2 force-pushed the lres_google branch from feaa04a to 668b144 Compare April 5, 2023 13:38

proppy reviewed Apr 5, 2023

View reviewed changes

klayout/drc/rule_decks/lres.drc Outdated Show resolved Hide resolved

Adding intermediate var for LRES table

81ec8f6

proppy merged commit 85a6957 into google:main Apr 6, 2023

atorkmabrains deleted the lres_google branch April 6, 2023 10:54

Adding LRES table for GF180MCU DRC #65

Adding LRES table for GF180MCU DRC #65

Conversation

FaragElsayed2 commented Apr 3, 2023

Adding LRES table for GF180MCU DRC

atorkmabrains commented Apr 3, 2023

atorkmabrains commented Apr 4, 2023

proppy commented Apr 4, 2023

atorkmabrains commented Apr 4, 2023

proppy commented Apr 4, 2023

atorkmabrains left a comment

Choose a reason for hiding this comment

atorkmabrains commented Apr 4, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

proppy Apr 5, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

proppy Apr 5, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

proppy Apr 5, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

proppy left a comment

Choose a reason for hiding this comment

atorkmabrains commented Apr 5, 2023

proppy commented Apr 6, 2023 • edited Loading

proppy Apr 5, 2023 •

edited

Loading

proppy Apr 5, 2023 •

edited

Loading

proppy Apr 5, 2023 •

edited

Loading

proppy commented Apr 6, 2023 •

edited

Loading