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.

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

Assertion failed '!"Unexpected tree op after call marked as tailcall"' #99798

Closed
MichalStrehovsky opened this issue Mar 15, 2024 · 3 comments · Fixed by #99806
Closed

Assertion failed '!"Unexpected tree op after call marked as tailcall"' #99798

MichalStrehovsky opened this issue Mar 15, 2024 · 3 comments · Fixed by #99806
Assignees
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Milestone

Comments

@MichalStrehovsky
Copy link
Member

Currently failing in main. To repro:

build clr.aot+libs.native+libs.sfx -rc Checked -lc Release
src\tests\build.cmd nativeaot checked tree managed

Then find find the failing ilc.exe command line in the build log if you need it under debugger (look for "exited with code").

JitDump:

dump.txt

Cc @jakobbotsch @EgorBo

@MichalStrehovsky MichalStrehovsky added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Mar 15, 2024
Copy link
Contributor

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Mar 15, 2024
@jakobbotsch
Copy link
Member

We have

------------ BB05 [0005] [000..001) -> BB06(1) (always), preds={BB01} succs={BB06}

***** BB05 [0005]
STMT00006 ( INL01 @ ??? ... ??? ) <- INLRT @ 0x000[E-]
               [000045] DACXG------                         *  STORE_LCL_VAR int    V06 tmp4         
               [000046] --CXG------                         \--*  CALL ind nullcheck int   
               [000047] ----------- this                       +--*  LCL_VAR   byref  V03 tmp1          (last use)
               [000044] ---XG------ gctx                       +--*  IND       long  
               [000043] -----------                            |  \--*  ADD       long  
               [000039] -----------                            |     +--*  SUB       long  
               [000040] -----------                            |     |  +--*  LCL_VAR   long   V05 tmp3         
               [000041] -----------                            |     |  \--*  CNS_INT   long   2
               [000042] -----------                            |     \--*  CNS_INT   long   8
               [000048] ----------- arg2                       +--*  LCL_VAR   ref    V01 arg1          (last use)
               [000038] ---XG------ calli tgt                  \--*  IND       long  
               [000037] -----------                               \--*  SUB       long  
               [000035] -----------                                  +--*  LCL_VAR   long   V05 tmp3          (last use)
               [000036] -----------                                  \--*  CNS_INT   long   2

------------ BB06 [0002] [000..00D) (return), preds={BB04,BB05} succs={}

***** BB06 [0002]
STMT00001 ( 0x000[E-] ... ??? )
               [000014] --C--------                         *  RETURN    int   
               [000023] -----------                         \--*  CAST      int <- ubyte <- int
               [000022] -----------                            \--*  LCL_VAR   int    V06 tmp4          (last use)

and [000046] is marked as a tailcall. That's not legal -- we will lose the truncating cast if we convert it to a tailcall.

It looks exposed by #99265.

@jakobbotsch jakobbotsch self-assigned this Mar 15, 2024
@jakobbotsch jakobbotsch added this to the 9.0.0 milestone Mar 15, 2024
@jakobbotsch jakobbotsch removed the untriaged New issue has not been triaged by the area owner label Mar 15, 2024
@jakobbotsch
Copy link
Member

Actually the call has gtReturnType == TYP_UBYTE, so getting rid of the cast is ok. I'll enhance things a bit so inlining doesn't insert the cast in the first place.

jakobbotsch added a commit to jakobbotsch/runtime that referenced this issue Mar 15, 2024
…zation in inlining

During call importation, for fat pointer calls we will introduce a local
and spill the call to it. This loses track of the small typedness of the
value, which can cause inlining to introduce unnecessary normalization
casts later. For tailcalls this can cause us to add IR after the call
that we do not expect, causing issues like dotnet#99798.

Fix the problem by enhancing logic in a few places:
- Make the local created for these fat pointer calls small typed like
  regular normalize-on-store locals
- Enhance `fgCastNeeded` to take into account the small-typedness of
  these locals (like `IntegralRange::ForNode`)
jakobbotsch added a commit that referenced this issue Mar 15, 2024
…zation in inlining (#99806)

During call importation, for fat pointer calls we will introduce a local
and spill the call to it. This loses track of the small typedness of the
value, which can cause inlining to introduce unnecessary normalization
casts later. For tailcalls this can cause us to add IR after the call
that we do not expect, causing issues like #99798.

Fix the problem by enhancing logic in a few places:
- Make the local created for these fat pointer calls small typed like
  regular normalize-on-store locals
- Enhance `fgCastNeeded` to take into account the small-typedness of
  these locals (like `IntegralRange::ForNode`)
@github-actions github-actions bot locked and limited conversation to collaborators Apr 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
2 participants