Skip to content

[S-TIR] Fix the ScheduleError from missing guard in CheckInline#19377

Merged
tlopex merged 3 commits intoapache:mainfrom
cchung100m:issue-18380
Apr 11, 2026
Merged

[S-TIR] Fix the ScheduleError from missing guard in CheckInline#19377
tlopex merged 3 commits intoapache:mainfrom
cchung100m:issue-18380

Conversation

@cchung100m
Copy link
Copy Markdown
Contributor

Hi Commiters,

This PR is trying to fix issues #18380. Any suggestions would be appreciated if you are available.

Root Cause

The ScheduleError comes from a missing guard in CheckInline before calling GetScopRoot

Solution

Skip Inline if this is the root block

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a check in AutoInlineNode::CheckInline to prevent the inlining of root blocks by returning InlineType::kNoInline if the block's parent reference is null. This ensures that blocks without a parent scope are correctly identified as non-inlinable. I have no feedback to provide as no review comments were submitted.

@cchung100m cchung100m marked this pull request as ready for review April 10, 2026 16:12
@tlopex
Copy link
Copy Markdown
Member

tlopex commented Apr 10, 2026

The current guard block_sref->parent == nullptr only handles the case where the block has no parent at all.

That is not sufficient here: if a root block is wrapped in a For loop, then parent is non-null, but there is still no enclosing SBlockNode. In that case, GetScopeRoot will still throw RootBlockError when it walks up the parent chain and fails to find an SBlockNode.

I think the guard should mirror the logic in GetScopeRoot, e.g. by traversing the parent chain and checking whether any ancestor is an SBlockNode. If none is found, return InlineType::kNoInline.

It would also be good to add a test where a root block is wrapped in a For loop, since the current check would miss that case.

  const StmtSRefNode* p = block_sref->parent;                   
  for (; p != nullptr; p = p->parent) {                                                   
    if (p->stmt->IsInstance<SBlockNode>()) break;
  }                                                                                       
  if (p == nullptr) {                                           
    return InlineType::kNoInline;
  }

Copy link
Copy Markdown
Member

@tlopex tlopex left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks

@tlopex tlopex merged commit 828880e into apache:main Apr 11, 2026
9 checks passed
@cchung100m cchung100m deleted the issue-18380 branch April 11, 2026 05:25
@cchung100m
Copy link
Copy Markdown
Contributor Author

Thanks to @tlopex for the prompt reply and suggestions. 😄

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.

2 participants