Skip to content

[ARM32] Display wrong register number for double registers #8121

Closed

Description

@hqueue

For ARM32, double register number is not shown correctly in JitDump, JitDisasm and etc.

  • An example from coreclr unit test.

    $ COMPlus_JitDisasm=DblAdd COMPlus_JitDump=DblAdd  ./corerun 
    tests/JIT/CodeGenBringUpTests/DblAdd/DblAdd.exe
    ...(omitted)...
    IN0001: 000006  EE30 0B01      vadd    d0, d0, d2
    ...(omitted)...
    IN0001: 000006      vadd    d0, d0, d2
    

    However above instruction encoding EE30 0B01 means vadd d0, d0, d1.

  • Another example from recent PR for ryujit/arm32.
    I was confused with below example ([RyuJIT/ARM32] Enable casting from small int to float coreclr#11643 (comment)) in the PR.

    IN0074: 000180  EEB8 4BC4      vcvt.i2d d8, s8  
    

    This encoding EEB8 4BC4 means vcvt.i2d d4, s8, i.e. single-float register number is correct, but double register number is incorrectly displayed.

Both LegacyJIT and RyuJIT have same problem.

category:implementation
theme:debug-dumps
skill-level:intermediate
cost:medium

Activity

  1. reopened this on May 16, 2017
  2. karelz commented on May 16, 2017

    @karelz
    Member

    My appologies, I moved the wrong bug by accident, reopening ...

  3. hqueue commented on May 17, 2017

    @hqueue
    MemberAuthor

    It seems to be not a trivial job to make it print out double registers for ARM32 correctly, because there are different paths to get a name of ARM32 double registers.

    • COMPlus_JitDump
      • Each phase use different method to print out register name.
    • COMPlus_JitDisasm

    For example, each phase of JIT compiler use different way to print out double registers, i.e. calling getRegNameFloat(), getRegName(), compRegVarName(), emitRegName() and etc.

    In addition, I've a concern that if I changed this, then it will introduce a lot of differences in jit-diff for ARM32 codegen.

    Do you have any idea ?

  4. BruceForstall commented on May 18, 2017

    @BruceForstall
    Contributor

    I think for arm32 legacy_backend, we just lived with it, since while it is confusing, once you understand it, there is no ambiguity. It would be nice to fix it, though.

    I've a concern that if I changed this, then it will introduce a lot of differences in jit-diff for ARM32 codegen.

    That is true, but would be a one-time, temporary thing.

  5. hqueue commented on May 18, 2017

    @hqueue
    MemberAuthor

    I think for arm32 legacy_backend, we just lived with it, since while it is confusing, once you understand it, there is no ambiguity.

    Right, if we are aware of it, there is no problem.

    Because there are other urgent issues in ryujit/arm32, I'm going to leave it as future task and just notify it to other arm32 contributors.

    @dotnet/arm32-contrib Please be aware that double register number in JitDump and JitDisam is not correct for ARM32 in both LegacyJIT and RyuJIT.

  6. BruceForstall commented on Aug 2, 2017

    @BruceForstall
    Contributor

    Since this isn't required to ship RyuJIT/ARM32, I'm removing it from the RyuJIT/ARM32 project.

  7. AndyAyersMS commented on Feb 23, 2018

    @AndyAyersMS
    Member

    @BruceForstall seems like this should be moved to Future...?

  8. BruceForstall commented on Feb 23, 2018

    @BruceForstall
    Contributor

    Yes. Done.

  9. transferred this issue fromdotnet/coreclron Jan 31, 2020
  10. added this to the Future milestone on Jan 31, 2020
  11. self-assigned this
    on Dec 23, 2022
  12. kunalspathak commented on Dec 23, 2022

    @kunalspathak
    Contributor

    I can confirm the problem still exists.

    ; Method getChars.MyXMLNodeWriter:DblAdd(double,double):double
    G_M34776_IG01:              ;; offset=0000H
    000000  E92D 4800      push    {r11,lr}
    000004  46EB           mov     r11, sp
    						;; size=6 bbWeight=1    PerfScore 2.00
    
    G_M34776_IG02:              ;; offset=0006H
    000006  EE30 0B01      vadd    d0, d0, d2
    						;; size=4 bbWeight=1    PerfScore 1.00
    
    G_M34776_IG03:              ;; offset=000AH
    00000A  E8BD 8800      pop     {r11,pc}
    						;; size=4 bbWeight=1    PerfScore 1.00
    ; Total bytes of code: 14
  13. ghost added
    in-prThere is an active PR which will close this issue when it is merged
    on Dec 24, 2022
  14. ghost removed
    in-prThere is an active PR which will close this issue when it is merged
    on Dec 28, 2022
  15. ghost locked as resolved and limited conversation to collaborators on Jan 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

arch-arm32area-CodeGen-coreclrCLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMIenhancementProduct code improvement that does NOT require public API changes/additions

Type

No type

Projects

No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions