Page MenuHomePhabricator

[ARM] Add 2-operand assembly aliases for Thumb1 ADD/SUB
ClosedPublic

Authored by olista01 on Sep 1 2017, 3:06 AM.

Details

Summary

This adds 2-operand assembly aliases for these instructions:

add r0, r1    =>   add r0, r0, r1
sub r0, r1    =>   sub r0, r0, r1

Previously this syntax was only accepted for Thumb2 targets, where the
wide versions of the instructions were used.

This patch allows the 2-operand syntax to be used for Thumb1 targets,
and selects the narrow encoding when it is used for Thumb2 targets.

Diff Detail

Repository
rL LLVM

Event Timeline

olista01 created this revision.Sep 1 2017, 3:06 AM
eastig accepted this revision.Sep 1 2017, 3:16 AM

LGTM.
BTW the issue was found during porting micropython for Cortex-M0 from GCC to Clang.

This revision is now accepted and ready to land.Sep 1 2017, 3:16 AM
This revision was automatically updated to reflect the committed changes.
rengolin edited edge metadata.Sep 1 2017, 3:55 AM

As I said in D37374, please refrain from approving your own patches within minutes of posting. The community must be part of the process, and if people are not reviewing your patches as fast as you want, then there's either something wrong with the community (and we need to fix), or with your expectations.

If the problem is in the community, then the documented approach to make people care about your work is to care about theirs first. The more you review other people's patches, the more likely they are to review yours. If that doesn't work (after months), then we should discuss the problems in the dev list.

eastig added a comment.Sep 1 2017, 5:55 AM

As I said in D37374, please refrain from approving your own patches within minutes of posting. The community must be part of the process, and if people are not reviewing your patches as fast as you want, then there's either something wrong with the community (and we need to fix), or with your expectations.

If the problem is in the community, then the documented approach to make people care about your work is to care about theirs first. The more you review other people's patches, the more likely they are to review yours. If that doesn't work (after months), then we should discuss the problems in the dev list.

Hi Renato,

I agree with you: other people from the community should also have a chance to review patches.

There might be my fault because I put my comment with the action 'Accept revision'. My understanding of the use of 'Accept revision' is that I have reviewed a patch and I accept it. Further steps, especially when to commit, should be done according to the LLVM developer policy. Maybe my understanding of this is wrong.

Sorry for the inconvenience caused.
-Evgeny

There might be my fault because I put my comment with the action 'Accept revision'. My understanding of the use of 'Accept revision' is that I have reviewed a patch and I accept it. Further steps, especially when to commit, should be done according to the LLVM developer policy. Maybe my understanding of this is wrong.

When you select "Accept" it means the patch can be committed without further reviews (unless you state clearly against it).

On same-company cases, due to conflict of interests as well as time-zone-wise, accepting a patch in less than 24-hours is really bad practice.

When in doubt, ask the code owner.

cheers,
--renato

eastig added a comment.Sep 1 2017, 6:53 AM

Thank you, Renato, for clarifying that.