Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

-DMLIR_ENABLE_EXPENSIVE_PATTERN_API_CHECKS=ON might have found some invalid API usages #7047

Open
7FM opened this issue May 16, 2024 · 2 comments
Open

-DMLIR_ENABLE_EXPENSIVE_PATTERN_API_CHECKS=ON might have found some invalid API usages #7047

7FM opened this issue May 16, 2024 · 2 comments
Labels
Arc Involving the `arc` dialect Calyx The Calyx dialect Comb Involving the `comb` dialect FIRRTL Involving the `firrtl` dialect Handshake help wanted Extra attention is needed HW Involving the `hw` dialect LLHD LoopSchedule

Comments

@7FM
Copy link
Contributor

7FM commented May 16, 2024

While the documentation states that there might be false positives, I think it might be worth a look into the individual test cases and patterns.
If there are no false positives, adding -DMLIR_ENABLE_EXPENSIVE_PATTERN_API_CHECKS=ON to the CI could be beneficial.

Failed Tests (32):
  CIRCT :: Conversion/LoopScheduleToCalyx/convert_pipeline.mlir
  CIRCT :: Conversion/SCFToCalyx/cider_source_location.mlir
  CIRCT :: Conversion/SCFToCalyx/convert_controlflow.mlir
  CIRCT :: Conversion/SCFToCalyx/convert_func.mlir
  CIRCT :: Conversion/SCFToCalyx/convert_memory.mlir
  CIRCT :: Conversion/SCFToCalyx/convert_simple.mlir
  CIRCT :: Conversion/SCFToCalyx/errors.mlir
  CIRCT :: Dialect/Arc/arc-canonicalizer.mlir
  CIRCT :: Dialect/Arc/canonicalizers.mlir
  CIRCT :: Dialect/Calyx/canonicalization.mlir
  CIRCT :: Dialect/Comb/canonicalization.mlir
  CIRCT :: Dialect/FIRRTL/SFCTests/GrandCentralInterfaces/Wire.fir
  CIRCT :: Dialect/FIRRTL/SFCTests/data-taps.fir
  CIRCT :: Dialect/FIRRTL/SFCTests/invalid-interpretations.fir
  CIRCT :: Dialect/FIRRTL/SFCTests/invalid-reg-pass.fir
  CIRCT :: Dialect/FIRRTL/SFCTests/mem-taps.fir
  CIRCT :: Dialect/FIRRTL/canonicalization.mlir
  CIRCT :: Dialect/FIRRTL/ref.mlir
  CIRCT :: Dialect/FIRRTL/simplify-mems.mlir
  CIRCT :: Dialect/HW/canonicalization.mlir
  CIRCT :: Dialect/HW/inline.mlir
  CIRCT :: Dialect/Handshake/canonicalization.mlir
  CIRCT :: Dialect/LLHD/Canonicalization/extract.mlir
  CIRCT :: firtool/async-reset.fir
  CIRCT :: firtool/chirrtl.fir
  CIRCT :: firtool/firtool.fir
  CIRCT :: firtool/import-ref.fir
  CIRCT :: firtool/lower-memories.fir
  CIRCT :: firtool/prefixMemory.fir
  CIRCT :: firtool/spec/refs/define.fir
  CIRCT :: firtool/spec/refs/nested_refproducer.fir
  CIRCT :: firtool/sv-attr.fir


Testing Time: 2.55s

Total Discovered Tests: 812
  Unsupported      :  12 (1.48%)
  Passed           : 762 (93.84%)
  Expectedly Failed:   6 (0.74%)
  Failed           :  32 (3.94%)

The observed errors are:

  • LLVM ERROR: operation finger print changed
  • LLVM ERROR: pattern returned success but IR did not change
  • LLVM ERROR: IR failed to verify after pattern application
@7FM 7FM added help wanted Extra attention is needed FIRRTL Involving the `firrtl` dialect Handshake HW Involving the `hw` dialect LLHD Calyx The Calyx dialect Comb Involving the `comb` dialect Arc Involving the `arc` dialect LoopSchedule labels May 16, 2024
@dtzSiFive
Copy link
Contributor

Thanks for trying this and reporting!

I'm seeing 34 errors presently on my setup, here's a log but I'll start going through these... https://gist.github.com/dtzSiFive/cc4c9652739f455bb36e36490c98d940 .

Enabling debug prints for pattern application (not included above) helps indicate what pattern caused the bug.

dtzSiFive added a commit to dtzSiFive/circt that referenced this issue May 16, 2024
@dtzSiFive
[SV][Folds] Fix ForOp canonicalizer to use rewriter.
930e2b3 
Detected by -DMLIR_ENABLE_EXPENSIVE_PATTERN_API_CHECKS=ON.
cc llvm#7047.
@dtzSiFive dtzSiFive mentioned this issue May 16, 2024
Merged
dtzSiFive added a commit that referenced this issue May 16, 2024
@dtzSiFive
[SV][Folds] Fix ForOp canonicalizer to use rewriter. (#7048)
831a466 
Detected by -DMLIR_ENABLE_EXPENSIVE_PATTERN_API_CHECKS=ON.
cc #7047.
dtzSiFive added a commit to dtzSiFive/circt that referenced this issue May 16, 2024
@dtzSiFive
[FIRRTL][Folds] Fix FoldUnusedPorts to use rewriter for RAUW.
7f2c895 
Detected by -DMLIR_ENABLE_EXPENSIVE_PATTERN_API_CHECKS=ON.

cc llvm#7047.
dtzSiFive added a commit to dtzSiFive/circt that referenced this issue May 16, 2024
@dtzSiFive
[FIRRTL][Folds] Fix FoldReadWritePorts to use rewriter for RAUW.
a15298b 
Detected by -DMLIR_ENABLE_EXPENSIVE_PATTERN_API_CHECKS=ON.

cc llvm#7047.
This was referenced May 16, 2024
Merged
Open
dtzSiFive added a commit to dtzSiFive/circt that referenced this issue May 16, 2024
@dtzSiFive
[Handshake] Fix canonicalizer not going through rewriter for RAUW.
00c516c 
Detected by -DMLIR_ENABLE_EXPENSIVE_PATTERN_API_CHECKS=ON.

cc llvm#7047.
@dtzSiFive dtzSiFive mentioned this issue May 16, 2024
Open
dtzSiFive added a commit to dtzSiFive/circt that referenced this issue May 16, 2024
@dtzSiFive
[HW] Fix pattern returning success() without changing.
ccc1562 
Detected by -DMLIR_ENABLE_EXPENSIVE_PATTERN_API_CHECKS=ON.

cc llvm#7047.
This was referenced May 16, 2024
Merged
Merged
dtzSiFive added a commit that referenced this issue May 16, 2024
@dtzSiFive
[HW] Fix pattern returning success() without changing. (#7053)
4f546dc 
Detected by -DMLIR_ENABLE_EXPENSIVE_PATTERN_API_CHECKS=ON.

cc #7047.
dtzSiFive added a commit to dtzSiFive/circt that referenced this issue May 16, 2024
@dtzSiFive
[LLHD] Fix operand assignments not going through rewriter.
8091658 
Detected by -DMLIR_ENABLE_EXPENSIVE_PATTERN_API_CHECKS=ON.

cc llvm#7047.
@dtzSiFive dtzSiFive mentioned this issue May 16, 2024
Merged
dtzSiFive added a commit to dtzSiFive/circt that referenced this issue May 16, 2024
@dtzSiFive
[FIRRTL] Fix width of attribute in MuxPadSel canonicalizations.
75cbad3 
Detected by -DMLIR_ENABLE_EXPENSIVE_PATTERN_API_CHECKS=ON.

cc llvm#7047.
@dtzSiFive dtzSiFive mentioned this issue May 16, 2024
Merged
dtzSiFive added a commit that referenced this issue May 16, 2024
@dtzSiFive
[LLHD] Fix operand assignments not going through rewriter. (#7055)
cb397bb 
Detected by -DMLIR_ENABLE_EXPENSIVE_PATTERN_API_CHECKS=ON.

cc #7047.
dtzSiFive added a commit that referenced this issue May 17, 2024
@dtzSiFive
[FIRRTL] Fix width of attribute in MuxPadSel canonicalizations. (#7056)
dc216e8 
Detected by -DMLIR_ENABLE_EXPENSIVE_PATTERN_API_CHECKS=ON.

cc #7047.
dtzSiFive added a commit that referenced this issue May 17, 2024
@dtzSiFive
[FIRRTL][Folds] Fix patterns to use rewriter for RAUW (#7049)
79148d8 
Fix FoldUnusedPorts to use rewriter for RAUW.
Fix FoldReadWritePorts to use rewriter for RAUW.

Detected by -DMLIR_ENABLE_EXPENSIVE_PATTERN_API_CHECKS=ON.

cc #7047.

Fix more RAUW's to use rewriter, inspection.
@dtzSiFive
Copy link
Contributor

This is now down to 11 -- 10 once the Handshake PR lands.

Here's the test list, FWIW:

Failed Tests (11):
  CIRCT :: Conversion/LoopScheduleToCalyx/convert_pipeline.mlir
  CIRCT :: Conversion/SCFToCalyx/cider_source_location.mlir
  CIRCT :: Conversion/SCFToCalyx/convert_controlflow.mlir
  CIRCT :: Conversion/SCFToCalyx/convert_func.mlir
  CIRCT :: Conversion/SCFToCalyx/convert_memory.mlir
  CIRCT :: Conversion/SCFToCalyx/convert_simple.mlir
  CIRCT :: Conversion/SCFToCalyx/errors.mlir
  CIRCT :: Dialect/Arc/arc-canonicalizer.mlir
  CIRCT :: Dialect/Arc/canonicalizers.mlir
  CIRCT :: Dialect/Calyx/canonicalization.mlir
  CIRCT :: Dialect/Handshake/canonicalization.mlir

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Arc Involving the `arc` dialect Calyx The Calyx dialect Comb Involving the `comb` dialect FIRRTL Involving the `firrtl` dialect Handshake help wanted Extra attention is needed HW Involving the `hw` dialect LLHD LoopSchedule
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@7FM @dtzSiFive