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.

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

[AVR] Optimize 16-bit comparisions with zero #30923

Closed
dylanmckay opened this issue Jan 8, 2017 · 7 comments
Closed

[AVR] Optimize 16-bit comparisions with zero #30923

dylanmckay opened this issue Jan 8, 2017 · 7 comments
Assignees
Labels
backend:AVR bugzilla Issues migrated from bugzilla

Comments

@dylanmckay
Copy link
Member

Bugzilla Link 31575
Version trunk
OS All

Extended Description

Currently when we try and perform a 16-bit comparision with zero, we generate the following code:

ldi r24, 0
ldi r25, 0
cp r26, r24
cpc r27, r25
breq LBB0_4

We can simplify this to

ldi r24, 0
cp r26, r24
cpc r27, r24

And save a register

@dylanmckay
Copy link
Member Author

That last snippet is missing a 'breq LBB0_4' at the bottom.

@llvmbot
Copy link
Member

llvmbot commented Jan 9, 2017

I have a few further suggestions:

  1. Instead of ldi, I think that we should use clr, so that any register can be initialized to zero. ldi is only allowed for registers r16 to r31.

  2. This can be generalized to other comparisons. For example, when comparing a 32-bit value to 0x11221122, we will only need to load two 8-bit immediate values 0x11 and 0x22. Or when comparing a 16-bit value to 0xffff or a 32-bit value to 0xffffffff, we only need one immediate value 0xff.

  3. For the least significant byte, cpi can be used if the register is between r16 and r31 (see item 1) and if that byte value is only needed for the LSB (see item 2).

  4. Equality comparisons == and != (as opposed to < > <= >=) could use cpse/rjmp instead of cpi/cp/cpc followed by a branch. This could allow simple interrupt handlers to avoid saving and restoring SREG.

@llvmbot
Copy link
Member

llvmbot commented Oct 10, 2018

Proposed patch
Hi,
Please find attached a patch which optimizes the 16-bit compare instruction.

Original generated code:
ldi r24, 0
ldi r25, 0
cp r18, r24
cpc r19, r25
breq LBB0_2

Updated Code:
cpi r24, 0
sbci r25, 0
breq LBB0_2

The patch uses an immediate compare using 'cpi' followed by a 'sbci' which takes into account the
generated carry.
We save 2 registers and 2 instructions in this case for any 16-bit compare.

Kindly review this patch and let me know if I have missed anything.
I have added a testcase for this fix,
test/CodeGen/AVR/#31575 .ll

Regression tested for targets which I built (avr, x86 and arm) on SVR revision 343839.
Testing Time: 295.82s
Expected Passes : 18919
Expected Failures : 61
Unsupported Tests : 9003
[100%] Built target check-llvm

I did observe a failure in todays build in testcase : CodeGen/AVR/call.ll
in revision 344047, however this seems unrelated to the patch as it fails with/without the patch.
I will file separate bug report for this.

Regards,
Kaushik

@dylanmckay
Copy link
Member Author

Great work Kaushik, thanks!

I will get onto reviewing the patch

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 10, 2021
@benshi001 benshi001 self-assigned this Jan 29, 2022
@benshi001
Copy link
Member

An alternative way is

cp  rlo, zero_reg
cpc rhi, zero_reg

The zero_reg is R17 on avrtiny or R1 on others.

@benshi001 benshi001 removed their assignment Jan 29, 2022
@benshi001 benshi001 self-assigned this Jul 25, 2022
@benshi001 benshi001 removed their assignment Aug 15, 2022
@benshi001
Copy link
Member

fixed by https://reviews.llvm.org/D142281

@benshi001 benshi001 self-assigned this Jan 21, 2023
@benshi001
Copy link
Member

fixed by f37d7c9

CarlosAlbertoEnciso pushed a commit to SNSystems/llvm-debuginfo-analyzer that referenced this issue Feb 9, 2023
veselypeta pushed a commit to veselypeta/cherillvm that referenced this issue Aug 9, 2024
veselypeta pushed a commit to veselypeta/cherillvm that referenced this issue Aug 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:AVR bugzilla Issues migrated from bugzilla
Projects
None yet
Development

No branches or pull requests

3 participants