Skip to content

Invalid instructions (UMAAL) are generated for the thumbv7m-none-eabi target #37227

Closed
@japaric

Description

@japaric
Member

UMAAL is only available in the ARMv7E-M processors but if you compile core for the thumbv7m (ARMv7-M) target, rustc/LLVM generates them for the core::num::flt2dec::strategy::grisu::format_shortest_opt function.

Activity

added
A-codegenArea: Code generation
O-ArmTarget: 32-bit Arm processors (armv6, armv7, thumb...), including 64-bit Arm in AArch32 state
on Oct 17, 2016
japaric

japaric commented on Oct 17, 2016

@japaric
MemberAuthor

Relevant information: See the ARM Cortex-M instructions groups table under instructions sets. UMAAL is in the group of instructions that are not available for the Cortex-M3, which is the processor the thumbv7m target covers.

Building core with a explicit -C target-cpu=cortex-m3 still generates the UMAAL instruction. This seems like a bug in LLVM.

added
A-LLVMArea: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues.
on Oct 17, 2016
pftbest

pftbest commented on Oct 18, 2016

@pftbest
Contributor

Minimal IR that triggers the problem:
bugpoint-reduced-simplified.ll
It generates umaal instruction when is compiled by llc

pftbest

pftbest commented on Oct 22, 2016

@pftbest
Contributor

I have submitted the fix for review:
https://reviews.llvm.org/D25890

pftbest

pftbest commented on Oct 27, 2016

@pftbest
Contributor

Fixed in llvm upstream, r285278 and r285280

pftbest

pftbest commented on Oct 28, 2016

@pftbest
Contributor

How does llvm version gets updated? Do I need to backport this fix to llvm 3.9 somehow, so it could land in rust?

alexcrichton

alexcrichton commented on Oct 28, 2016

@alexcrichton
Member

@pftbest nice! To update LLVM there's two routes:

  1. Backport the patches to our fork, currently around the 3.9 release.
  2. Upgrade our fork to the current LLVM head

(1) is much easier than (2) typically, and we're fine with both!

pftbest

pftbest commented on Oct 28, 2016

@pftbest
Contributor

So, I've rebased rust-llvm-2016-07-09 branch to match llvm version 3.9.1 and then I backported my fix on top of it. The new branch is called rust-llvm-2016-10-29 and I don't know what to do next.

japaric

japaric commented on Oct 28, 2016

@japaric
MemberAuthor

@pftbest I think:

  • Locally, make the rust repo's llvm submodule point to your 10-29 branch and test (make check) locally.
  • If that works, ask a core member someone with push access to the rust-lang/llvm repo to create a new 10-29 branch there
  • Then send another PR to rust-lang/rust updating its llvm submodule to the new branch in rust-lang/llvm
  • Cross your fingers and hope that it passes the buildbots.
alexcrichton

alexcrichton commented on Oct 29, 2016

@alexcrichton
Member

Ok I've pushed your branch to rust-lang/llvm (thanks for the rebase to 3.9.1!). As @japaric mentioned if you want to send a PR to rust-lang/rust updating the submodule I'll r+ that

pftbest

pftbest commented on Oct 29, 2016

@pftbest
Contributor

Thank you! I have submitted a #37465

2 remaining items

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-LLVMArea: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues.A-codegenArea: Code generationO-ArmTarget: 32-bit Arm processors (armv6, armv7, thumb...), including 64-bit Arm in AArch32 state

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @alexcrichton@pftbest@japaric

        Issue actions

          Invalid instructions (UMAAL) are generated for the thumbv7m-none-eabi target · Issue #37227 · rust-lang/rust