Skip to content

[TVMScript] Do not throw error for duplicate definitions - #16811

Merged
Lunderberg merged 1 commit into
apache:mainfrom
Lunderberg:tvmscript_printer_must_handle_non_ssa
Apr 3, 2024
Merged

Lunderberg merged 1 commit into
apache:mainfrom
Lunderberg:tvmscript_printer_must_handle_non_ssa

Conversation

@Lunderberg

Copy link
Copy Markdown
Contributor

TVM's IR dialects require single-site assignment. However, the printer is different from most utilities, as it is often used for debugging when runtime assumptions are violated. Therefore, it may neither assume that its input is well-formed, nor may it throw an exception if the input is ill-formed, and must print an accurate representation of the underlying IR for use in debugging. When debugging, being unable to print an ill-formed IRModule prevents a developer from determining why the IRModule is ill-formed.

TVM's IR dialects require single-site assignment. However, the printer
is different from most utilities, as it may neither assume that its
input is well-formed, nor may it throw an exception if the input is
ill-formed.  The printer is often used for debugging, where logging
and printouts of an IRModule are essential.  In these cases, throwing
an error would prevent a developer from determining why an IRModule is
ill-formed.
@Lunderberg
Lunderberg force-pushed the tvmscript_printer_must_handle_non_ssa branch from 139b923 to d56237b Compare March 29, 2024 12:55

@slyubomirsky slyubomirsky left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good idea and good point about treating the printer as a debugging tool, though I think logging a warning or some kind of indication that something is wrong would be useful to add.

@Lunderberg

Copy link
Copy Markdown
Contributor Author

I went back and forth on it, as the existing ICHECK could have been changed to a LOG(WARNING). However, that would prevent it from generating clean error messages for cases that are known to be ill-formed. (e.g. Highlighting the ill-formed output, as it done for the TIR well-formed checker and which I want to bring over to the Relax well-formed checker.)

@slyubomirsky

Copy link
Copy Markdown
Contributor

Glad to hear there's a plan.

@Lunderberg
Lunderberg merged commit 35c6143 into apache:main Apr 3, 2024
@Lunderberg
Lunderberg deleted the tvmscript_printer_must_handle_non_ssa branch April 3, 2024 15:12
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