circt icon indicating copy to clipboard operation
circt copied to clipboard

[FIRRTL] Windows build broken

Open teqdruid opened this issue 1 year ago • 35 comments

First failure: https://github.com/llvm/circt/actions/runs/7143564112/job/19455325008

FAIL: CIRCT :: Dialect/FIRRTL/SFCTests/mem-taps-reg.fir (331 of 678)
******************** TEST 'CIRCT :: Dialect/FIRRTL/SFCTests/mem-taps-reg.fir' FAILED ********************
Exit Code: 2

Command Output (stdout):
--
# RUN: at line 1
d:\a\circt\circt\build\bin\firtool.exe --verilog D:\a\circt\circt\test\Dialect\FIRRTL\SFCTests\mem-taps-reg.fir | d:\a\circt\circt\build\bin\filecheck.exe D:\a\circt\circt\test\Dialect\FIRRTL\SFCTests\mem-taps-reg.fir
# executed command: 'd:\a\circt\circt\build\bin\firtool.exe' --verilog 'D:\a\circt\circt\test\Dialect\FIRRTL\SFCTests\mem-taps-reg.fir'
# .---command stderr------------
# | PLEASE submit a bug report to https://github.com/llvm/circt and include the crash backtrace.
# | Stack dump:
# | 0.	Program arguments: d:\\a\\circt\\circt\\build\\bin\\firtool.exe --verilog D:\\a\\circt\\circt\\test\\Dialect\\FIRRTL\\SFCTests\\mem-taps-reg.fir
# | Exception Code: 0xC0000005
# |  #0 0x00007ff75e5bbdb4 (d:\a\circt\circt\build\bin\firtool.exe+0x1fbdb4)
# |  #1 0x00007ff75e5c1f74 (d:\a\circt\circt\build\bin\firtool.exe+0x201f74)
# |  #2 0x00007ff75ee37af2 (d:\a\circt\circt\build\bin\firtool.exe+0xa77af2)
# |  #3 0x00007ff75ee3d162 (d:\a\circt\circt\build\bin\firtool.exe+0xa7d162)
# |  #4 0x00007ff75ee3eda1 (d:\a\circt\circt\build\bin\firtool.exe+0xa7eda1)
# |  #5 0x00007ff75ee38084 (d:\a\circt\circt\build\bin\firtool.exe+0xa78084)
# |  #6 0x00007ff75ee3e745 (d:\a\circt\circt\build\bin\firtool.exe+0xa7e745)
# |  #7 0x00007ff75ee37b87 (d:\a\circt\circt\build\bin\firtool.exe+0xa77b87)
# |  #8 0x00007ff75ee3d162 (d:\a\circt\circt\build\bin\firtool.exe+0xa7d162)
# |  #9 0x00007ff75ee3d91e (d:\a\circt\circt\build\bin\firtool.exe+0xa7d91e)
# | #10 0x00007ff75e3ce516 (d:\a\circt\circt\build\bin\firtool.exe+0xe516)
# | #11 0x00007ff75e3ce9b9 (d:\a\circt\circt\build\bin\firtool.exe+0xe9b9)
# | #12 0x00007ff75e3cc9f7 (d:\a\circt\circt\build\bin\firtool.exe+0xc9f7)
# | #13 0x00007ff75e3d0f1a (d:\a\circt\circt\build\bin\firtool.exe+0x10f1a)
# | #14 0x00007ff75ee56f20 (d:\a\circt\circt\build\bin\firtool.exe+0xa96f20)
# | #15 0x00007ffe1e644de0 (C:\Windows\System32\KERNEL32.DLL+0x14de0)
# | #16 0x00007ffe1ec3ed9b (C:\Windows\SYSTEM32\ntdll.dll+0x7ed9b)
# `-----------------------------
# error: command failed with exit status: 0xc0000005
# executed command: 'd:\a\circt\circt\build\bin\filecheck.exe' 'D:\a\circt\circt\test\Dialect\FIRRTL\SFCTests\mem-taps-reg.fir'
# .---command stderr------------
# | FileCheck error: '<stdin>' is empty.
# | FileCheck command line:  d:\a\circt\circt\build\bin\filecheck.exe D:\a\circt\circt\test\Dialect\FIRRTL\SFCTests\mem-taps-reg.fir
# `-----------------------------
# error: command failed with exit status: 2

--

********************
Testing:  0.. 10.. 20.. 30.. 40..
FAIL: CIRCT :: Dialect/FIRRTL/SFCTests/mem-taps.fir (332 of 678)
******************** TEST 'CIRCT :: Dialect/FIRRTL/SFCTests/mem-taps.fir' FAILED ********************
Exit Code: 2

Command Output (stdout):
--
# RUN: at line 1
d:\a\circt\circt\build\bin\firtool.exe --verilog D:\a\circt\circt\test\Dialect\FIRRTL\SFCTests\mem-taps.fir | d:\a\circt\circt\build\bin\filecheck.exe D:\a\circt\circt\test\Dialect\FIRRTL\SFCTests\mem-taps.fir
# executed command: 'd:\a\circt\circt\build\bin\firtool.exe' --verilog 'D:\a\circt\circt\test\Dialect\FIRRTL\SFCTests\mem-taps.fir'
# .---command stderr------------
# | PLEASE submit a bug report to https://github.com/llvm/circt and include the crash backtrace.
# | Stack dump:
# | 0.	Program arguments: d:\\a\\circt\\circt\\build\\bin\\firtool.exe --verilog D:\\a\\circt\\circt\\test\\Dialect\\FIRRTL\\SFCTests\\mem-taps.fir
# | Exception Code: 0xC0000005
# |  #0 0x00007ff75e5bbdb4 (d:\a\circt\circt\build\bin\firtool.exe+0x1fbdb4)
# |  #1 0x00007ff75e5c1f74 (d:\a\circt\circt\build\bin\firtool.exe+0x201f74)
# |  #2 0x00007ff75ee37af2 (d:\a\circt\circt\build\bin\firtool.exe+0xa77af2)
# |  #3 0x00007ff75ee3d162 (d:\a\circt\circt\build\bin\firtool.exe+0xa7d162)
# |  #4 0x00007ff75ee3eda1 (d:\a\circt\circt\build\bin\firtool.exe+0xa7eda1)
# |  #5 0x00007ff75ee38084 (d:\a\circt\circt\build\bin\firtool.exe+0xa78084)
# |  #6 0x00007ff75ee3e745 (d:\a\circt\circt\build\bin\firtool.exe+0xa7e745)
# |  #7 0x00007ff75ee37b87 (d:\a\circt\circt\build\bin\firtool.exe+0xa77b87)
# |  #8 0x00007ff75ee3d162 (d:\a\circt\circt\build\bin\firtool.exe+0xa7d162)
# |  #9 0x00007ff75ee3d91e (d:\a\circt\circt\build\bin\firtool.exe+0xa7d91e)
# | #10 0x00007ff75e3ce516 (d:\a\circt\circt\build\bin\firtool.exe+0xe516)
# | #11 0x00007ff75e3ce9b9 (d:\a\circt\circt\build\bin\firtool.exe+0xe9b9)
# | #12 0x00007ff75e3cc9f7 (d:\a\circt\circt\build\bin\firtool.exe+0xc9f7)
# | #13 0x00007ff75e3d0f1a (d:\a\circt\circt\build\bin\firtool.exe+0x10f1a)
# | #14 0x00007ff75ee56f20 (d:\a\circt\circt\build\bin\firtool.exe+0xa96f20)
# | #15 0x00007ffe1e644de0 (C:\Windows\System32\KERNEL32.DLL+0x14de0)
# | #16 0x00007ffe1ec3ed9b (C:\Windows\SYSTEM32\ntdll.dll+0x7ed9b)
# `-----------------------------
# error: command failed with exit status: 0xc0000005
# executed command: 'd:\a\circt\circt\build\bin\filecheck.exe' 'D:\a\circt\circt\test\Dialect\FIRRTL\SFCTests\mem-taps.fir'
# .---command stderr------------
# | FileCheck error: '<stdin>' is empty.
# | FileCheck command line:  d:\a\circt\circt\build\bin\filecheck.exe D:\a\circt\circt\test\Dialect\FIRRTL\SFCTests\mem-taps.fir
# `-----------------------------
# error: command failed with exit status: 2

--

********************
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90.. 
********************
Failed Tests (2):
  CIRCT :: Dialect/FIRRTL/SFCTests/mem-taps-reg.fir
  CIRCT :: Dialect/FIRRTL/SFCTests/mem-taps.fir

teqdruid avatar Dec 11 '23 21:12 teqdruid

Yea, been working on getting a windows build for 2 days. might finally have it.

On Mon, Dec 11, 2023 at 4:00 PM John Demme @.***> wrote:

Assigned #6513 https://github.com/llvm/circt/issues/6513 to @darthscsi https://github.com/darthscsi.

— Reply to this email directly, view it on GitHub https://github.com/llvm/circt/issues/6513#event-11216320546, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACY5KKZHF5W7QCBZ6DLA6DLYI566XAVCNFSM6AAAAABAQOYKTOVHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJRGIYTMMZSGA2TINQ . You are receiving this because you were assigned.Message ID: @.***>

darthscsi avatar Dec 11 '23 23:12 darthscsi

Annoyingly, a vs2022 build doesn't see the problem.

darthscsi avatar Dec 12 '23 02:12 darthscsi

Testers use 14.37.32822, locally with 14.38.33130 it passes.

darthscsi avatar Dec 12 '23 02:12 darthscsi

Can't reproduce locally with the same compiler.

darthscsi avatar Dec 12 '23 18:12 darthscsi

@darthscsi If you don't mind, I'd like to help look into it this week.

SpriteOvO avatar Dec 12 '23 18:12 SpriteOvO

By all means.

darthscsi avatar Dec 12 '23 21:12 darthscsi

My attempt at reproduction was with a debug build while the tester is a release build, I'm trying that variation now.

darthscsi avatar Dec 12 '23 21:12 darthscsi

A release build fails whereas a debug build didn't. So replicated.

darthscsi avatar Dec 12 '23 21:12 darthscsi

Seems to actually correlate with a windows runner update: Image: windows-2022 Version: 20231205.1.0 v.s. Image: windows-2022 Version: 20231126.1.0

darthscsi avatar Dec 14 '23 17:12 darthscsi

Visual Studio Enterprise 2022 17.8.34330.188 v.s. Visual Studio Enterprise 2022 17.8.34316.72

darthscsi avatar Dec 14 '23 17:12 darthscsi

Given that this is clean in valgrind, ubsan, release and debug on all tested linux compilers, debug msvc, and only appears to be affected in release builds in one point release of MSVC, I think this needs a more windows knowledgeable person to look like.

darthscsi avatar Dec 14 '23 17:12 darthscsi

I tried Visual Studio 2022 17.8.1 for both commit 5838a9e, which is the last version passing windows build, and the next commit 0d48f71. 5838a9e can pass all tests, while 0d48f71 failed just like the CI. The commands I ran are identical to what CI does, except that I did not use sccache.

Visual Studio 2022 17.8.1 is version 17.8.34316.72 accroding to Visual Studio 2022 Release History, which was used in windows runner version 20231126.1.0.

Just FYI :) @darthscsi

Tang-Haojin avatar Dec 21 '23 13:12 Tang-Haojin

I installed vs2022 build tools through this link, which lies in Visual Studio 2022 Release History.

Tang-Haojin avatar Dec 21 '23 13:12 Tang-Haojin

I've looked into it tonight, the crash is occurred at

https://github.com/llvm/circt/blob/0d48f71a6aead69f81b7f507159e04ff3267137a/lib/Dialect/FIRRTL/Transforms/Dedup.cpp#L1405

the crash callstack is

>	firtool.exe!fixupAllModules(circt::firrtl::InstanceGraph & instanceGraph) Line 1405	C++
 	firtool.exe!`anonymous namespace'::DedupPass::runOnOperation() Line 1680	C++
 	[Inline Frame] firtool.exe!mlir::detail::OpToOpPassAdaptor::run::__l2::<lambda_2>::operator()() Line 503	C++
 	firtool.exe!llvm::function_ref<void __cdecl(void)>::callback_fn<`mlir::detail::OpToOpPassAdaptor::run'::`2'::<lambda_2>>(__int64 callable) Line 45	C++
 	[Inline Frame] firtool.exe!llvm::function_ref<void __cdecl(void)>::operator()() Line 68	C++
 	[Inline Frame] firtool.exe!mlir::MLIRContext::executeAction(llvm::function_ref<void __cdecl(void)>) Line 275	C++
 	firtool.exe!mlir::detail::OpToOpPassAdaptor::run(mlir::Pass * pass, mlir::Operation * op, mlir::AnalysisManager am, bool verifyPasses, unsigned int parentInitGeneration) Line 509	C++
 	firtool.exe!mlir::detail::OpToOpPassAdaptor::runPipeline(mlir::OpPassManager & pm, mlir::Operation * op, mlir::AnalysisManager am, bool verifyPasses, unsigned int parentInitGeneration, mlir::PassInstrumentor * instrumentor, const mlir::PassInstrumentation::PipelineParentInfo * parentInfo) Line 569	C++
 	[Inline Frame] firtool.exe!mlir::detail::OpToOpPassAdaptor::runOnOperationAsyncImpl::__l2::<lambda_1>::operator()(mlir::detail::OpToOpPassAdaptor::runOnOperationAsyncImpl::__l2::OpPMInfo & opInfo) Line 789	C++
 	firtool.exe!mlir::failableParallelForEach<std::_Vector_iterator<std::_Vector_val<std::_Simple_types<`mlir::detail::OpToOpPassAdaptor::runOnOperationAsyncImpl'::`2'::OpPMInfo>>>,`mlir::detail::OpToOpPassAdaptor::runOnOperationAsyncImpl'::`2'::<lambda_1> &>(mlir::MLIRContext * context, std::_Vector_iterator<std::_Vector_val<std::_Simple_types<`mlir::detail::OpToOpPassAdaptor::runOnOperationAsyncImpl'::`2'::OpPMInfo>>> begin, std::_Vector_iterator<std::_Vector_val<std::_Simple_types<`mlir::detail::OpToOpPassAdaptor::runOnOperationAsyncImpl'::`2'::OpPMInfo>>> end, mlir::detail::OpToOpPassAdaptor::runOnOperationAsyncImpl::__l2::<lambda_1> & func) Line 46	C++
 	firtool.exe!mlir::failableParallelForEach<std::vector<`mlir::detail::OpToOpPassAdaptor::runOnOperationAsyncImpl'::`2'::OpPMInfo,std::allocator<`mlir::detail::OpToOpPassAdaptor::runOnOperationAsyncImpl'::`2'::OpPMInfo>> &,`mlir::detail::OpToOpPassAdaptor::runOnOperationAsyncImpl'::`2'::<lambda_1> &>(mlir::MLIRContext * context, std::vector<`mlir::detail::OpToOpPassAdaptor::runOnOperationAsyncImpl'::`2'::OpPMInfo,std::allocator<`mlir::detail::OpToOpPassAdaptor::runOnOperationAsyncImpl'::`2'::OpPMInfo>> & range, mlir::detail::OpToOpPassAdaptor::runOnOperationAsyncImpl::__l2::<lambda_1> & func) Line 92	C++
 	firtool.exe!mlir::detail::OpToOpPassAdaptor::runOnOperationAsyncImpl(bool verifyPasses) Line 799	C++
 	[Inline Frame] firtool.exe!mlir::detail::OpToOpPassAdaptor::run::__l2::<lambda_2>::operator()() Line 501	C++
 	firtool.exe!llvm::function_ref<void __cdecl(void)>::callback_fn<`mlir::detail::OpToOpPassAdaptor::run'::`2'::<lambda_2>>(__int64 callable) Line 45	C++
 	[Inline Frame] firtool.exe!llvm::function_ref<void __cdecl(void)>::operator()() Line 68	C++
 	[Inline Frame] firtool.exe!mlir::MLIRContext::executeAction(llvm::function_ref<void __cdecl(void)>) Line 275	C++
 	firtool.exe!mlir::detail::OpToOpPassAdaptor::run(mlir::Pass * pass, mlir::Operation * op, mlir::AnalysisManager am, bool verifyPasses, unsigned int parentInitGeneration) Line 509	C++
 	firtool.exe!mlir::detail::OpToOpPassAdaptor::runPipeline(mlir::OpPassManager & pm, mlir::Operation * op, mlir::AnalysisManager am, bool verifyPasses, unsigned int parentInitGeneration, mlir::PassInstrumentor * instrumentor, const mlir::PassInstrumentation::PipelineParentInfo * parentInfo) Line 569	C++
 	firtool.exe!mlir::PassManager::runPasses(mlir::Operation * op, mlir::AnalysisManager am) Line 880	C++
 	firtool.exe!mlir::PassManager::run(mlir::Operation * op) Line 859	C++
 	firtool.exe!processBuffer(mlir::MLIRContext & context, circt::firtool::FirtoolOptions & firtoolOptions, mlir::TimingScope & ts, llvm::SourceMgr & sourceMgr, std::optional<std::unique_ptr<llvm::ToolOutputFile,std::default_delete<llvm::ToolOutputFile>>> & outputFile) Line 464	C++
 	firtool.exe!processInputSplit(mlir::MLIRContext & context, circt::firtool::FirtoolOptions & firtoolOptions, mlir::TimingScope & ts, std::unique_ptr<llvm::MemoryBuffer,std::default_delete<llvm::MemoryBuffer>> buffer, std::optional<std::unique_ptr<llvm::ToolOutputFile,std::default_delete<llvm::ToolOutputFile>>> & outputFile) Line 536	C++
 	firtool.exe!processInput(mlir::MLIRContext & context, circt::firtool::FirtoolOptions & firtoolOptions, mlir::TimingScope & ts, std::unique_ptr<llvm::MemoryBuffer,std::default_delete<llvm::MemoryBuffer>> input, std::optional<std::unique_ptr<llvm::ToolOutputFile,std::default_delete<llvm::ToolOutputFile>>> & outputFile) Line 575	C++
 	firtool.exe!executeFirtool(mlir::MLIRContext & context, circt::firtool::FirtoolOptions & firtoolOptions) Line 643	C++
 	firtool.exe!main(int argc, char * * argv) Line 732	C++
 	[External Code]	

where the result points to an invalid memory address.


I have a suspicion that this piece of code is corrupting the pointer of the result of the instance, for unknown reasons.

https://github.com/llvm/circt/blob/0d48f71a6aead69f81b7f507159e04ff3267137a/lib/Dialect/FIRRTL/Transforms/LowerSignatures.cpp#L396-L428

I will continue to try to uncover the cause in the next.

SpriteOvO avatar Dec 21 '23 20:12 SpriteOvO

Any further progress on this? It's blocking my pycde releases (since I gate the releases on passing all of the tests on both Windows and Linux). I'd like to get new releases sometime next week.

teqdruid avatar Dec 29 '23 06:12 teqdruid

@darthscsi If I simply revert 0d48f71a6aead69f81b7f507159e04ff3267137a, the build passes: https://github.com/llvm/circt/actions/runs/7354417130/job/20021597121.

teqdruid avatar Dec 29 '23 06:12 teqdruid

Any further progress on this? It's blocking my pycde releases (since I gate the releases on passing all of the tests on both Windows and Linux). I'd like to get new releases sometime next week.

I apologize, I have a higher priority task right now, so I have to defer looking into this issue :(

But it's probably safe to assume that it's either due to LowerSignaturesPass containing implementation-defined behavior or some sort of compiler bug, since it all works fine on Linux.

SpriteOvO avatar Dec 30 '23 06:12 SpriteOvO

As near as I can tell this is tickling a compiler bug (and I don't say this lightly) in one point release of msvc, which also changed in the runners at that commit. The code is ubsan, adresssan, valgrind clean. It passes on debug builds for that msvc. It passes on other points releases of msvc. It passes on other msvc major releases. It passes on all clang and gcc versions I've tried.

On Fri, Dec 29, 2023, 1:55 AM John Demme @.***> wrote:

If I simply revert 0d48f71 https://github.com/llvm/circt/commit/0d48f71a6aead69f81b7f507159e04ff3267137a, the build passes: https://github.com/llvm/circt/actions/runs/7354417130/job/20021597121.

— Reply to this email directly, view it on GitHub https://github.com/llvm/circt/issues/6513#issuecomment-1871793744, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACY5KK5KWJ2Y25TYACCXUIDYLZSOLAVCNFSM6AAAAABAQOYKTOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNZRG44TGNZUGQ . You are receiving this because you were mentioned.Message ID: @.***>

darthscsi avatar Dec 30 '23 13:12 darthscsi

The responsible thing to do is to report this to the MSVC team. Ideally, we'd have a small repro to report -- all of LLVM + CIRCT would be too much.

teqdruid avatar Jan 02 '24 19:01 teqdruid

**********************************************************************
** Visual Studio 2022 Developer PowerShell v17.7.7
** Copyright (c) 2022 Microsoft Corporation
**********************************************************************
$ cl
Microsoft (R) C/C++ Optimizing Compiler Version 19.37.32826.1 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.
********************
Failed Tests (2):
  CIRCT :: Dialect/FIRRTL/SFCTests/mem-taps-reg.fir
  CIRCT :: Dialect/FIRRTL/SFCTests/mem-taps.fir

If it's really a compiler bug, it's also present in 2022 v17.7.7.

teqdruid avatar Jan 02 '24 21:01 teqdruid

It also fails in my internal pipeline with VS 2019:

**********************************************************************
** Visual Studio 2019 Developer PowerShell v16.11.32
** Copyright (c) 2021 Microsoft Corporation
**********************************************************************

I don't think this is a compiler bug. Certainly not related to the point release bump.

teqdruid avatar Jan 02 '24 22:01 teqdruid

Thanks for all the reports, folks!

FWIW we switched to the 2022 image specifically to avoid some failures, see: https://github.com/llvm/circt/issues/6395 https://github.com/llvm/circt/issues/6193

I just noticed our release artifacts are built with 2019, let's move this to 2022 to match what we test in CI: https://github.com/llvm/circt/pull/6548 .

dtzSiFive avatar Jan 04 '24 17:01 dtzSiFive

@dtzSiFive Switching to 2022 didn't help.

teqdruid avatar Jan 04 '24 19:01 teqdruid

@dtzSiFive Switching to 2022 didn't help.

Understood. Wanted the two (CI + Release flow) on same runner for its own sake. FWIW 2019 -> 2022 appears to have avoided some failures in the past but maybe it's just moving things around-- at this point I'm not sure what's going on re:crashes in Windows builds.

On the 2019 runner, the release flow saw different tests crash (FWIW):

 Failed Tests (5):
  CIRCT :: Dialect/FIRRTL/SFCTests/parameters.fir
  CIRCT :: firtool/firtool.fir
  CIRCT :: firtool/instchoice.fir
  CIRCT :: firtool/spec/refs/forwarding_refs_upwards.fir
  CIRCT :: ibistool/high_level.mlir

dtzSiFive avatar Jan 05 '24 15:01 dtzSiFive

Doesn't change the decided plan (work on reducing the failure, XFAIL on Windows the tests in the short-term as needed) probably but found this interesting:

2022 tests that fail on master (this issue's subject) apparently pass in the release flow, which does run the tests, they just passed for firtool-1.62.0: https://github.com/llvm/circt/actions/runs/7474243016/job/20339999283 . Haven't looked into what the differences may be but interesting! :smile: .

dtzSiFive avatar Jan 10 '24 22:01 dtzSiFive

We've got two PRs, both of which unbreak the build: #6570 and #6571 (thanks to @fzi-hielscher). @darthscsi / @seldridge -- which one would you be more comfortable with?

The latter "fixes" the problem, but since we don't understand why it "fixes" the problem, I'm not convinced that we don't have a deeper problem. It probably just changes msvc code emission slightly to cover up a problem. So some other innocuous change (code, msvc flags, msvc version, etc.) could bring up the same problem.

So it may be more appropriate to XFAIL the tests in recognition that we don't really understand the issue.

Your choice. I don't much care either way since (IMO) they're both half solutions.

teqdruid avatar Jan 11 '24 02:01 teqdruid

Given @seldridge's comment in #6571, I merged that one.

I think we should keep this issue open for further investigation later on... When we have a stake holder who cares about FIRRTL on Windows.

teqdruid avatar Jan 11 '24 02:01 teqdruid

I kicked off a bunch of Windows builds off of main and they all passed. So, this at least appears to be deterministically passing (as it seemed to be deterministically failing before). There's clearly still a problem. However, I think this has nothing to do with the original @darthscsi commit causing this to "fail". It's some problem related to how MSVC is compiling this which may be in CIRCT. If it is in CIRCT it may be in dedup, but possibly in an earlier pass which creates some bad state and is only exposed because dedup examines basically everything in the complete MLIR description. 🥲

seldridge avatar Jan 11 '24 03:01 seldridge

I might find the time and motivation to actually look at the (dis)assembly and figure out what's going on there later this week, but I cannot promise anything. If someone wants to pick up where I'll leave it for now: The snippet below is sufficient to trigger the error on my system even with all other passes before ProbeDCE disabled. It appears to always crash at the seventh result of the instance operation, but as shown by the PR even minor changes to the for loop can make the problem disappear.

firrtl.circuit "Top" {
  firrtl.module private @DUTModule(in %clock: !firrtl.clock, in %reset: !firrtl.reset, out %bar0: !firrtl.uint<1>, out %bar1: !firrtl.uint<1>, out %bar2: !firrtl.uint<1>, out %bar3: !firrtl.uint<1>, out %bar4: !firrtl.uint<1>) {

  }
  firrtl.module @Top(in %clock: !firrtl.clock, in %reset: !firrtl.uint<1>) {
    %dut_clock, %dut_reset, %bar0, %bar1, %bar2, %bar3, %bar4 = firrtl.instance dut interesting_name @DUTModule(in clock: !firrtl.clock, in reset: !firrtl.reset, out bar0: !firrtl.uint<1>, out bar1: !firrtl.uint<1>, out bar2: !firrtl.uint<1>, out bar3: !firrtl.uint<1>, out bar4: !firrtl.uint<1>)
    %8 = firrtl.resetCast %reset : (!firrtl.uint<1>) -> !firrtl.reset
    firrtl.strictconnect %dut_reset, %8 : !firrtl.reset
  }
}

fzi-hielscher avatar Jan 11 '24 03:01 fzi-hielscher

I found that simply change unsigned to size_t can unbreak this issue on my local machine:

diff --git a/lib/Dialect/FIRRTL/Transforms/Dedup.cpp b/lib/Dialect/FIRRTL/Transforms/Dedup.cpp
index dc244a26d..1750509bb 100644
--- a/lib/Dialect/FIRRTL/Transforms/Dedup.cpp
+++ b/lib/Dialect/FIRRTL/Transforms/Dedup.cpp
@@ -1399,7 +1399,7 @@ void fixupAllModules(InstanceGraph &instanceGraph) {
         continue;
       ImplicitLocOpBuilder builder(inst.getLoc(), inst->getContext());
       builder.setInsertionPointAfter(inst);
-      for (unsigned i = 0, e = getNumPorts(module); i < e; ++i) {
+      for (size_t i = 0, e = getNumPorts(module); i < e; ++i) {
         auto result = inst.getResult(i);
         auto newType = module.getPortType(i);
         auto oldType = result.getType();

Tang-Haojin avatar Jan 11 '24 03:01 Tang-Haojin