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.

Sign up for GitHub

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

Jump to bottom

Better host handling in CompilationConfig & debug printing #9460

Merged
merged 1 commit into from
Nov 9, 2021
Merged

Better host handling in CompilationConfig & debug printing #9460

merged 1 commit into from
Nov 9, 2021

Conversation

mbs-octoml
Copy link
Contributor

@mbs-octoml mbs-octoml commented Nov 5, 2021 •
edited
Loading

(This is a bit of a grab bag in preparation for #9326
which I'm trying to minimize)

While switching the device planner to use SEScopes I had a lot
of trouble with Target's not matching up.

  • If no explicit host target is given but the given
    TargetMap has targets with hosts, try to use those
    to establish the host_target.
  • Make sure both the 'legacy' TargetMap representation
    and the newer representation agree to pointer equality on
    their targets.
  • Make sure the Interpreter uses the target from CompilationConfig
    since it's been normalized.

To debug the above:

  • When in pretty printing with show_meta_data_ false give as much
    detail on SEScopes, Targets and call attributes as possible.
    That needed some rework in the relay_text_printer.cc.
  • Ditto for critical 'target' attribute on PrimFuncs.
  • Also added a Target::ToDebugString so I could see the
    host fields along with everything else since a lot of problems
    were caused by a mismatch of 'the same' Target with and without
    a host. (Tried using that for the ReprPrinter but broken unit
    tests.)

Note that the codebase assumes Targets are compared by ObjectPtrEquality,
yet CheckAndUpdateHostConsistency (I count 65 call sites) changes the targets.
Ultimately CompilationConfig or it's ultimate replacement should ensure we munge
targets only once at the 'main' entry points.

@mbs-octoml
Better host handling in CompilationConfig & debug printing
a00f8aa 
(This is a bit of a grab bag in preparation for apache#9326
which I'm trying to minimize)

While switching the device planner to use SEScopes I had a lot
of trouble with Target's not matching up.
- If no explicit host target is given but the given
  TargetMap has targets with hosts, try to use those
  to establish the host_target.
- Make sure both the 'legacy' TargetMap representation
  and the newer representation agree to pointer equality on
  their targets.
- Make sure the Interpreter uses the target from CompilationConfig
  since it's been normalized.

To debug the above:
- When in pretty printing with show_meta_data_ false give as much
  detail on SEScopes, Targets and call attributes as possible.
  That needed some rework in the relay_text_printer.cc.
- Ditto for critical 'target' attribute on PrimFuncs.
- Also added a Target::ToDebugString so I could see the
  host fields along with everything else since a lot of problems
  were caused by a mismatch of 'the same' Target with and without
  a host. (Tried using that for the ReprPrinter but broken unit
  tests.)

Note that the codebase assumes Targets are compared by ObjectPtrEquality,
yet CheckAndUpdateHostConsistency (I count 65 call sites) changes the targets.
Ultimately CompilationConfig or it's ultimate replacement should ensure we munge
targets only once at the 'main' entry points.
@mbs-octoml mbs-octoml changed the title Better host handling in CompilationConfig etc Better host handling in CompilationConfig & debug printing Nov 6, 2021
electriclilies
electriclilies approved these changes Nov 8, 2021
View reviewed changes
Copy link
Contributor

@electriclilies electriclilies left a comment

Choose a reason for hiding this comment

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

LGTM after a quick look!

@TejashShah
Copy link

cc @jroesch @areusch Please help this PR get through.

@areusch areusch merged commit 675f348 into apache:main Nov 9, 2021
@mbs-octoml mbs-octoml deleted the mbs-pre-scopes branch November 9, 2021 17:19
AndrewZhaoLuo added a commit to AndrewZhaoLuo/tvm that referenced this pull request Nov 12, 2021
@AndrewZhaoLuo
Merge branch 'main' into aluo/qnn/conv2d-transpose-fq2i
da7a16b 
* main: (119 commits)
  [Topi][Op][PyTorch][Vitas] Fix inconsistent kernel layout conventions for conv2d_transpose (apache#9336)
  Fix repository URL in ubuntu_install_rocm.sh (apache#9425)
  Add LLVM-13 installation to Docker setup (apache#9498)
  [Relay] Use target_host determined at Relay level instead of recalculating it (apache#9499)
  Arm(R) Ethos(TM)-U NPU BinaryElementwise operators support (apache#9442)
  [COMMUNITY] Junru's and Wuwei's PGP key for ASF release (apache#9488)
  Add default for split op (apache#9489)
  [HOTFIX][TARGET] Change LOG in compilation config to DLOG (apache#9486)
  Fixed some warnings about lambda's closures that are bigger than necessary (apache#9481)
  [Support] Add libinfo into the runtime build (apache#9310)
  Change Call with TIRCallAttrs to call_lowered op (apache#9312)
  [ETHOSN] Streamline Ethos(TM)-N cross-compile rpc usage (apache#9477)
  [CMSIS-NN] Assert correct amount of CMSIS-NN artifacts in MLF (apache#9480)
  [MicroTVM][PyTest] Explicitly skip MicroTVM unittests. (apache#9335)
  [microNPU] Replace ICHECK with diagnostic context in type inference (apache#9470)
  Better host handling in CompilationConfig & debug printing (apache#9460)
  [AOT][Tests] Use pre-built libraries in Reference System tests (apache#9271)
  [TIR] Add type hint for TIR  (apache#9432)
  [TVMC] Add test for quantized pytorch model (apache#9467)
  [CMSIS-NN] Convert CMSIS-NN to use Target Hooks (apache#9397)
  ...
mehrdadh pushed a commit to mehrdadh/tvm that referenced this pull request Dec 1, 2021
@mbs-octoml @mehrdadh
Better host handling in CompilationConfig & debug printing (apache#9460)
e9404be 
(This is a bit of a grab bag in preparation for apache#9326
which I'm trying to minimize)

While switching the device planner to use SEScopes I had a lot
of trouble with Target's not matching up.
- If no explicit host target is given but the given
  TargetMap has targets with hosts, try to use those
  to establish the host_target.
- Make sure both the 'legacy' TargetMap representation
  and the newer representation agree to pointer equality on
  their targets.
- Make sure the Interpreter uses the target from CompilationConfig
  since it's been normalized.

To debug the above:
- When in pretty printing with show_meta_data_ false give as much
  detail on SEScopes, Targets and call attributes as possible.
  That needed some rework in the relay_text_printer.cc.
- Ditto for critical 'target' attribute on PrimFuncs.
- Also added a Target::ToDebugString so I could see the
  host fields along with everything else since a lot of problems
  were caused by a mismatch of 'the same' Target with and without
  a host. (Tried using that for the ReprPrinter but broken unit
  tests.)

Note that the codebase assumes Targets are compared by ObjectPtrEquality,
yet CheckAndUpdateHostConsistency (I count 65 call sites) changes the targets.
Ultimately CompilationConfig or it's ultimate replacement should ensure we munge
targets only once at the 'main' entry points.
mehrdadh pushed a commit to mehrdadh/tvm that referenced this pull request Dec 1, 2021
@mbs-octoml @mehrdadh
Better host handling in CompilationConfig & debug printing (apache#9460)
4140d07 
(This is a bit of a grab bag in preparation for apache#9326
which I'm trying to minimize)

While switching the device planner to use SEScopes I had a lot
of trouble with Target's not matching up.
- If no explicit host target is given but the given
  TargetMap has targets with hosts, try to use those
  to establish the host_target.
- Make sure both the 'legacy' TargetMap representation
  and the newer representation agree to pointer equality on
  their targets.
- Make sure the Interpreter uses the target from CompilationConfig
  since it's been normalized.

To debug the above:
- When in pretty printing with show_meta_data_ false give as much
  detail on SEScopes, Targets and call attributes as possible.
  That needed some rework in the relay_text_printer.cc.
- Ditto for critical 'target' attribute on PrimFuncs.
- Also added a Target::ToDebugString so I could see the
  host fields along with everything else since a lot of problems
  were caused by a mismatch of 'the same' Target with and without
  a host. (Tried using that for the ReprPrinter but broken unit
  tests.)

Note that the codebase assumes Targets are compared by ObjectPtrEquality,
yet CheckAndUpdateHostConsistency (I count 65 call sites) changes the targets.
Ultimately CompilationConfig or it's ultimate replacement should ensure we munge
targets only once at the 'main' entry points.
ylc pushed a commit to ylc/tvm that referenced this pull request Jan 7, 2022
@mbs-octoml @ylc
Better host handling in CompilationConfig & debug printing (apache#9460)
b3e6636 
(This is a bit of a grab bag in preparation for apache#9326
which I'm trying to minimize)

While switching the device planner to use SEScopes I had a lot
of trouble with Target's not matching up.
- If no explicit host target is given but the given
  TargetMap has targets with hosts, try to use those
  to establish the host_target.
- Make sure both the 'legacy' TargetMap representation
  and the newer representation agree to pointer equality on
  their targets.
- Make sure the Interpreter uses the target from CompilationConfig
  since it's been normalized.

To debug the above:
- When in pretty printing with show_meta_data_ false give as much
  detail on SEScopes, Targets and call attributes as possible.
  That needed some rework in the relay_text_printer.cc.
- Ditto for critical 'target' attribute on PrimFuncs.
- Also added a Target::ToDebugString so I could see the
  host fields along with everything else since a lot of problems
  were caused by a mismatch of 'the same' Target with and without
  a host. (Tried using that for the ReprPrinter but broken unit
  tests.)

Note that the codebase assumes Targets are compared by ObjectPtrEquality,
yet CheckAndUpdateHostConsistency (I count 65 call sites) changes the targets.
Ultimately CompilationConfig or it's ultimate replacement should ensure we munge
targets only once at the 'main' entry points.
yangulei pushed a commit to yangulei/tvm that referenced this pull request Jan 11, 2022
@mbs-octoml @yangulei
Better host handling in CompilationConfig & debug printing (apache#9460)
479ec76 
(This is a bit of a grab bag in preparation for apache#9326
which I'm trying to minimize)

While switching the device planner to use SEScopes I had a lot
of trouble with Target's not matching up.
- If no explicit host target is given but the given
  TargetMap has targets with hosts, try to use those
  to establish the host_target.
- Make sure both the 'legacy' TargetMap representation
  and the newer representation agree to pointer equality on
  their targets.
- Make sure the Interpreter uses the target from CompilationConfig
  since it's been normalized.

To debug the above:
- When in pretty printing with show_meta_data_ false give as much
  detail on SEScopes, Targets and call attributes as possible.
  That needed some rework in the relay_text_printer.cc.
- Ditto for critical 'target' attribute on PrimFuncs.
- Also added a Target::ToDebugString so I could see the
  host fields along with everything else since a lot of problems
  were caused by a mismatch of 'the same' Target with and without
  a host. (Tried using that for the ReprPrinter but broken unit
  tests.)

Note that the codebase assumes Targets are compared by ObjectPtrEquality,
yet CheckAndUpdateHostConsistency (I count 65 call sites) changes the targets.
Ultimately CompilationConfig or it's ultimate replacement should ensure we munge
targets only once at the 'main' entry points.
ylc pushed a commit to ylc/tvm that referenced this pull request Jan 13, 2022
@mbs-octoml @ylc
Better host handling in CompilationConfig & debug printing (apache#9460)
d05a441 
(This is a bit of a grab bag in preparation for apache#9326
which I'm trying to minimize)

While switching the device planner to use SEScopes I had a lot
of trouble with Target's not matching up.
- If no explicit host target is given but the given
  TargetMap has targets with hosts, try to use those
  to establish the host_target.
- Make sure both the 'legacy' TargetMap representation
  and the newer representation agree to pointer equality on
  their targets.
- Make sure the Interpreter uses the target from CompilationConfig
  since it's been normalized.

To debug the above:
- When in pretty printing with show_meta_data_ false give as much
  detail on SEScopes, Targets and call attributes as possible.
  That needed some rework in the relay_text_printer.cc.
- Ditto for critical 'target' attribute on PrimFuncs.
- Also added a Target::ToDebugString so I could see the
  host fields along with everything else since a lot of problems
  were caused by a mismatch of 'the same' Target with and without
  a host. (Tried using that for the ReprPrinter but broken unit
  tests.)

Note that the codebase assumes Targets are compared by ObjectPtrEquality,
yet CheckAndUpdateHostConsistency (I count 65 call sites) changes the targets.
Ultimately CompilationConfig or it's ultimate replacement should ensure we munge
targets only once at the 'main' entry points.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@electriclilies electriclilies electriclilies approved these changes

@areusch areusch areusch approved these changes

@anijain2305 anijain2305 Awaiting requested review from anijain2305

@comaniac comaniac Awaiting requested review from comaniac

@icemelon icemelon Awaiting requested review from icemelon

@jroesch jroesch Awaiting requested review from jroesch

@junrushao junrushao Awaiting requested review from junrushao

@jwfromm jwfromm Awaiting requested review from jwfromm

@kparzysz-quic kparzysz-quic Awaiting requested review from kparzysz-quic

@MarisaKirisame MarisaKirisame Awaiting requested review from MarisaKirisame

@masahi masahi Awaiting requested review from masahi

@mbrookhart mbrookhart Awaiting requested review from mbrookhart

@merrymercy merrymercy Awaiting requested review from merrymercy

@slyubomirsky slyubomirsky Awaiting requested review from slyubomirsky

@tqchen tqchen Awaiting requested review from tqchen

@vinx13 vinx13 Awaiting requested review from vinx13

@wweic wweic Awaiting requested review from wweic

@yzhliu yzhliu Awaiting requested review from yzhliu

@zhiics zhiics Awaiting requested review from zhiics

@ZihengJiang ZihengJiang Awaiting requested review from ZihengJiang

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@mbs-octoml @TejashShah @areusch @electriclilies