Skip to content

[SYCL]Fix bug: objdump find symbol error on adrp instruction when imm < 0 in arm64 #2500

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

Closed
wants to merge 1 commit into from

Conversation

JuunChen
Copy link

@JuunChen JuunChen commented Sep 20, 2020

Problem

image

I used the same llvm-objdump -S -m --section=<section name> <macho-file> command for two different macho files to see the disassembly code.
But the annotation for the assembly instruction can't show for the second file, as shown on the right.

Here is the two files which you can test:
the two macho files.zip

Reason

I found this to be a bug if the adrp's imm < 0, at the followed lines:

 adrp_imm = ((info->adrp_inst & 0x00ffffe0) >> 3) | ((info->adrp_inst >> 29) & 0x3);
 if (info->adrp_inst & 0x0200000)
      adrp_imm |= 0xfffffffffc000000LL;

The line adrp_imm = ((info->adrp_inst & 0x00ffffe0) >> 3) | ((info->adrp_inst >> 29) & 0x3) find the adrp_imm in adrp_inst, it's right.
And the next two lines are intended to:

  1. Determine if adrp_imm is a negative number
  2. if the adrp_imm is negative , adrp_imms's 64-bit complement is calculated

However, as shown in the picture:
image
adrp_imm is encoded as [23:5][31:29].

If you want to determine if adrp_imm is a negative number, you should determine the 23rd bit of adrp_inst, or the 20th bit of adrp_imm.
It will not be info->adrp_inst & 0x0200000. The 0x0200000 is 0b00000000001000000000000000000000,the code is to determine the 21st of adrp_inst,so it doesn't make any sense.
This code adrp_imm |= 0xfffffffffc000000LL is also wrong, it caculated the wrong bits.

Fix

Use the follow code to fix:

if (adrp_imm & (1 << (21 - 1)))
       adrp_imm |= ~((1LL << 21) - 1);

validation

After the correction, I got the result I wanted.

Differential Revision: https://reviews.llvm.org/D87985

@JuunChen JuunChen requested a review from bader as a code owner September 20, 2020 11:07
@bader
Copy link
Contributor

bader commented Sep 25, 2020

@JuunChen, sorry for the delay.
I think you are aiming to commit this change to llvm/llvm-project repository, rather than intel/llvm. Right?
If so, please, follow the process described here: https://llvm.org/docs/Contributing.html

@bader bader closed this Sep 27, 2020
jsji pushed a commit that referenced this pull request Apr 18, 2024
…ptoui and fptosi intrinsics (#2500)

* Regularize LLVM code to remove use of non standard integer types in fptoui and fptosi intrinsics

Signed-off-by: Sudarsanam, Arvind <[email protected]>

Original commit:
KhronosGroup/SPIRV-LLVM-Translator@14301c295d3dc8e
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