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.

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

Segfault when running firtool 1.60.0, standalone example included #6557

Open
oharboe opened this issue Jan 8, 2024 · 5 comments
Open

Segfault when running firtool 1.60.0, standalone example included #6557

oharboe opened this issue Jan 8, 2024 · 5 comments

Comments

@oharboe
Copy link

oharboe commented Jan 8, 2024

$ firtool --version
LLVM (http://llvm.org/):
  LLVM version 18.0.0git
  Optimized build.
CIRCT firtool-1.60.0

untar firtool-crash.tar.gz

Run:

firtool       --format=fir    --export-module-hierarchy       --verify-each=true    --warn-on-unprocessed-annotations       --disable-annotation-classless  --disable-annotation-unknown    --mlir-timing         --lowering-options=emittedLineLength=2048,noAlwaysComb,disallowLocalVariables,verifLabels,locationInfoStyle=wrapInAtSquareBracket,disallowPackedArrays,omitVersionComment     --repl-seq-mem-file=megaboom/mems.conf  --annotation-file=anno.json   --strip-debug-info   --strip-fir-debug-info   --extract-test-code       --split-verilog     --disable-all-randomization         -o megaboom/    ChipLikeMegaBoomConfig.sfc.fir

Output:

ChipLikeMegaBoomConfig.sfc.fir:2:1: warning: unprocessed annotation:'firrtl.transforms.NoCircuitDedupAnnotation$' still remaining after LowerToHW
circuit TestHarness :
^
ChipLikeMegaBoomConfig.sfc.fir:2:1: warning: unprocessed annotation:'logger.LogLevelAnnotation' still remaining after LowerToHW
circuit TestHarness :
^
ChipLikeMegaBoomConfig.sfc.fir:43120:3: warning: unprocessed annotation:'freechips.rocketchip.util.RegFieldDescMappingAnnotation' still remaining after LowerToHW
  module PeripheryBus :
  ^
ChipLikeMegaBoomConfig.sfc.fir:282740:3: warning: unprocessed annotation:'freechips.rocketchip.util.RetimeModuleAnnotation' still remaining after LowerToHW
  module IntToFP :
  ^
ChipLikeMegaBoomConfig.sfc.fir:557900:3: warning: unprocessed annotation:'freechips.rocketchip.util.ParamsAnnotation' still remaining after LowerToHW
  module BoomTile :
  ^
ChipLikeMegaBoomConfig.sfc.fir:696542:3: warning: unprocessed annotation:'freechips.rocketchip.util.AddressMapAnnotation' still remaining after LowerToHW
  module DigitalTop :
  ^
generators/rocket-chip/src/main/scala/util/DescribedSRAM.scala:17:26: warning: unprocessed annotation:'freechips.rocketchip.util.SRAMAnnotation' still remaining after LowerToHW
PLEASE submit a bug report to https://github.com/llvm/circt and include the crash backtrace.
Segmentation fault (core dumped)
@oharboe
Copy link
Author

oharboe commented Jan 8, 2024

Same crash with:

$ firtool --version
LLVM (http://llvm.org/):
  LLVM version 18.0.0git
  Optimized build with assertions.
CIRCT firtool-1.61.0

@oharboe
Copy link
Author

oharboe commented Jan 8, 2024

Without --extract-test-code, it doesn't crash.

@yupferris
Copy link
Contributor

yupferris commented Jan 8, 2024

I was able to reproduce in the debugger on current main (3ce5c8c). Looks like a null reference in mlir::Operation::getLoc from this call (introduced in #6402). I haven't narrowed down too much, but poking around it seems that it's trying to lower an array index op where the array is a module input, which is a case not covered in #6402 (and wasn't covered before that PR, either).

@yupferris
Copy link
Contributor

yupferris commented Jan 8, 2024

I can trigger the same error with the following MLIR:

hw.module @hi(in %arg0: !hw.array<2xi8>, in %sel: i1, out a: i8) {
  %0 = hw.array_get %arg0[%sel] : !hw.array<2xi8>, i1
  hw.output %0 : i8
}
$ firtool --format=mlir --verilog --lowering-options=disallowPackedArrays hi.mlir

I'm fairly certain this is representative of the original case.

The fix should be to lower packed array module inputs into individual inputs for each array element, and fix up module instantiations accordingly (similar treatment for outputs would also fix this test case). This is a bit more involved than the work I did previously on this code, so I wouldn't consider it a "quick fix" (for me at least), but doable in a couple days, I think. I am not currently working on this and I don't plan to at this time.

@uenoku
Copy link
Member

uenoku commented Jan 10, 2024

Looks like a null reference in mlir::Operation::getLoc from this call

We can avoid the failure by just calling input.getLoc().

The fix should be to lower packed array module inputs into individual inputs for each array element, and fix up module instantiations accordingly (similar treatment for outputs would also fix this test case)

I agree we can flatten IO in LegalizeModules but it would be very complicated to do that in that pass. I think extending FlattenIO is suitable for that purpose.

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

No branches or pull requests

3 participants