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

tlbilxlpid -> error: invalid instruction #64080

Closed
nickdesaulniers opened this issue Jul 24, 2023 · 12 comments · Fixed by llvm/llvm-project-release-prs#518
Closed

tlbilxlpid -> error: invalid instruction #64080

nickdesaulniers opened this issue Jul 24, 2023 · 12 comments · Fixed by llvm/llvm-project-release-prs#518
Assignees
@ecnelises
Labels
backend:PowerPC release:backport release:merged
Milestone
LLVM 17.0.X Release

Comments

@nickdesaulniers
Copy link
Member

via https://lore.kernel.org/llvm/202307211945.TSPcyOhh-lkp@intel.com/

it looks like:

void foo(void) {
    asm volatile("tlbilxlpid");
}

produces:

<source>:2:18: error: invalid instruction
    asm volatile("tlbilxlpid");
                 ^
<inline asm>:1:2: note: instantiated into assembly here
        tlbilxlpid
        ^~~~~~~~~~

for --target=powerpc-linux-gnu. I wonder if clang is missing support for this instruction (or perhaps it's an alias to something that clang does understand).

cc @nemanjai @nemanja-ibm
@nickdesaulniers nickdesaulniers mentioned this issue Jul 24, 2023
Closed
@llvmbot
Copy link
Collaborator

llvmbot commented Jul 24, 2023

@llvm/issue-subscribers-backend-powerpc

@MaskRay
Copy link
Member

MaskRay commented Jul 24, 2023

It looks like tlbilxlpid is an alias for tlbilx 0, 0, 0, which is present in Power ISA v2.07, but not in Power ISA v3.x.

@ecnelises ecnelises self-assigned this Jul 25, 2023
@nemanjai
Copy link
Member

Yes, the PowerISA has removed the Book E as of v3.0. @nickdesaulniers can you comment on where this is coming from? We don't have this implemented but if it is needed, we can go ahead with https://reviews.llvm.org/D156204.

@ecnelises
Copy link
Member

Landed as 53648ac

Should we backport it to 17.x?

@nickdesaulniers
Copy link
Member Author

Yes, please. We can work around this in the kernel using .inst assembler directives for older versions of clang, but having it sooner would allow us to drop the workaround sooner.

@nickdesaulniers
Copy link
Member Author

nickdesaulniers commented Aug 2, 2023 •
edited

oh no, is .inst aarch64-only? :(

EDIT: oh I can use .long here.

@ecnelises
Copy link
Member

/cherry-pick 53648ac

@llvmbot
Copy link
Collaborator

llvmbot commented Aug 3, 2023

/cherry-pick 53648ac

Error: Command failed due to missing milestone.

@ecnelises ecnelises added this to the LLVM 17.0.X Release milestone Aug 3, 2023
@ecnelises
Copy link
Member

/cherry-pick 53648ac

@llvmbot
Copy link
Collaborator

llvmbot commented Aug 3, 2023

/branch llvm/llvm-project-release-prs/issue64080

@llvmbot llvmbot mentioned this issue Aug 3, 2023
Merged
@llvmbot
Copy link
Collaborator

llvmbot commented Aug 3, 2023

/pull-request llvm/llvm-project-release-prs#518

@nickdesaulniers
Copy link
Member Author

Thanks @ecnelises !

@nickdesaulniers nickdesaulniers mentioned this issue Aug 3, 2023
Open
mpe pushed a commit to linuxppc/linux-ci that referenced this issue Aug 15, 2023
@nickdesaulniers @mpe
powerpc/inst: add PPC_TLBILX_LPID
4020dfd 
Clang didn't recognize the instruction tlbilxlpid. This was fixed in
clang-18 [0] then backported to clang-17 [1].  To support clang-16 and
older, rather than using that instruction bare in inline asm, add it to
ppc-opcode.h and use that macro as is done elsewhere for other
instructions.

Link: ClangBuiltLinux/linux#1891
Link: llvm/llvm-project#64080
Link: llvm/llvm-project@53648ac [0]
Link: llvm/llvm-project-release-prs@0af7e5e [1]
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/llvm/202307211945.TSPcyOhh-lkp@intel.com/
Suggested-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://msgid.link/20230803-ppc_tlbilxlpid-v3-1-ca84739bfd73@google.com
mpe pushed a commit to linuxppc/linux that referenced this issue Aug 17, 2023
@nickdesaulniers @mpe
powerpc/inst: add PPC_TLBILX_LPID
ae7936d 
Clang didn't recognize the instruction tlbilxlpid. This was fixed in
clang-18 [0] then backported to clang-17 [1].  To support clang-16 and
older, rather than using that instruction bare in inline asm, add it to
ppc-opcode.h and use that macro as is done elsewhere for other
instructions.

Link: ClangBuiltLinux#1891
Link: llvm/llvm-project#64080
Link: llvm/llvm-project@53648ac [0]
Link: llvm/llvm-project-release-prs@0af7e5e [1]
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/llvm/202307211945.TSPcyOhh-lkp@intel.com/
Suggested-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://msgid.link/20230803-ppc_tlbilxlpid-v3-1-ca84739bfd73@google.com
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ecnelises ecnelises

Labels
backend:PowerPC release:backport release:merged
Projects
LLVM Release Status
Status: Done
Milestone
LLVM 17.0.X Release
Development

Successfully merging a pull request may close this issue.

PR for llvm/llvm-project#64080 llvm/llvm-project-release-prs
6 participants
@MaskRay @nickdesaulniers @EugeneZelenko @ecnelises @nemanjai @llvmbot