Skip to content

Allow consecutive loops - #1943

Open
caleridas wants to merge 1 commit into
masterfrom
local-hls-fix
Open

caleridas wants to merge 1 commit into
masterfrom
local-hls-fix

Conversation

@caleridas

Copy link
Copy Markdown
Collaborator

Allow hls loops to be consecutive without memory merge operation in-between

@caleridas
caleridas requested review from phate and sjalander October 6, 2026 04:43
@caleridas

Copy link
Copy Markdown
Collaborator Author

This fixes at least one test for me locally for the hls test suite.

HOWEVER hls test suite still remains broken at master with a number of failures
(segfaults). In this state, I am unable to diagnose any other hls failures that
I may or may not introduce with changes as it is fully broken at master already.

@caleridas

Copy link
Copy Markdown
Collaborator Author

The next crash is in mem-conv.cpp, here:

if (branchOperation)
{
  // end of loop
  JLM_ASSERT(branchOperation->loop);
  state_edge = get_mem_state_user(
      util::assertedCast<rvsdg::RegionResult>(get_mem_state_user(sn->output(0)))->output());
}

the problem is that the assertion that a "region result" must necessarily follow a branch operation seems to be false -- in the crashing case, the branch op is not followed by a region result

I lack a comprehensive description of expected shape invariants, who is supposed to establish them, or what precisely the behavior of the code here should be. Obviously there are some strong assumptions of redundant structures (branch must be last op in region, and it must only exist there -- why is that the case, and if that is the case why are they needed to begin with? aren't they redundant?)

@sjalander

This branch has not been deployed

No deployments
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

Successfully merging this pull request may close these issues.

1 participant