Skip to content
This repository has been archived by the owner on Aug 20, 2024. It is now read-only.

ConstantPropagation: make RemoveValidIf an optional dependency #2027

Merged
merged 3 commits into from
Feb 1, 2021

Conversation

ekiwi
Copy link
Contributor

@ekiwi ekiwi commented Dec 22, 2020

This allows ConstantPropagation to be used in cases where ValidIfs need to be maintained, e.g., in the formal backend.

First step towards: #2011

Contributor Checklist

  • Did you specify the type of improvement?
  • Did you state the API impact?
  • Did you specify the code generation impact?
  • Did you request a desired merge strategy?

Type of Improvement

  • new feature/API

API Impact

This allows the ConstantPropagation pass to be scheduled without RemoveValidIf.

Backend Code Generation Impact

  • so far none, since RemoveValidIf will still be included in the default firrtl compiler.

Desired Merge Strategy

  • squash

Reviewer Checklist (only modified by reviewer)

  • Did you add the appropriate labels?
  • Did you mark the proper milestone (1.2.x, 1.3.0, 1.4.0) ?
  • Did you review?
  • Did you check whether all relevant Contributor checkboxes have been checked?
  • Did you mark as Please Merge?

This allows ConstantPropagation to be used in cases where
ValidIfs need to be maintained, e.g., in the formal backend.
Copy link
Member

@seldridge seldridge left a comment

Choose a reason for hiding this comment

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

This makes sense. There isn't a requirement that ConstantPropagation has to have RemoveValidIf. I expect this was like this just because it produce the "correct" ordering.

Note: it may be more accurate to express this as an invalidation from RemoveValidIf if the relationship is one of RemoveValidIf exposing more work for ConstantPropagation.

@jackkoenig jackkoenig added this to the 1.2.X milestone Jan 11, 2021
@jackkoenig jackkoenig added Please Merge Accepted PRs that are ready to be merged. Useful when waiting on CI. and removed Please Merge Accepted PRs that are ready to be merged. Useful when waiting on CI. labels Jan 11, 2021
@azidar azidar added the Please Merge Accepted PRs that are ready to be merged. Useful when waiting on CI. label Jan 25, 2021
@jackkoenig jackkoenig added Please Merge Accepted PRs that are ready to be merged. Useful when waiting on CI. and removed Please Merge Accepted PRs that are ready to be merged. Useful when waiting on CI. labels Jan 25, 2021
@mergify mergify bot merged commit ad0fd65 into chipsalliance:master Feb 1, 2021
mergify bot pushed a commit that referenced this pull request Feb 1, 2021
This allows ConstantPropagation to be used in cases where
ValidIfs need to be maintained, e.g., in the formal backend.

Co-authored-by: Adam Izraelevitz <[email protected]>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
(cherry picked from commit ad0fd65)

# Conflicts:
#	src/main/scala/firrtl/transforms/ConstantPropagation.scala
mergify bot pushed a commit that referenced this pull request Feb 1, 2021
This allows ConstantPropagation to be used in cases where
ValidIfs need to be maintained, e.g., in the formal backend.

Co-authored-by: Adam Izraelevitz <[email protected]>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
(cherry picked from commit ad0fd65)

# Conflicts:
#	src/main/scala/firrtl/transforms/ConstantPropagation.scala
mergify bot pushed a commit that referenced this pull request Feb 1, 2021
This allows ConstantPropagation to be used in cases where
ValidIfs need to be maintained, e.g., in the formal backend.

Co-authored-by: Adam Izraelevitz <[email protected]>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
(cherry picked from commit ad0fd65)
@mergify mergify bot added the Backported This PR has been backported to marked stable branch label Feb 1, 2021
mergify bot added a commit that referenced this pull request Feb 1, 2021
#2066)

This allows ConstantPropagation to be used in cases where
ValidIfs need to be maintained, e.g., in the formal backend.

Co-authored-by: Adam Izraelevitz <[email protected]>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
(cherry picked from commit ad0fd65)

Co-authored-by: Kevin Laeufer <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Backported This PR has been backported to marked stable branch Please Merge Accepted PRs that are ready to be merged. Useful when waiting on CI.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants