Skip to content

[AVR][MC] Fix incorrect range of relative jumps #109124

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

Merged
merged 1 commit into from
Sep 20, 2024
Merged

Conversation

benshi001
Copy link
Member

'rjmp .+4094' is legal but rejected by llvm-mc since 86a60e7, and this patch fixed that range issue.

'rjmp .+4094' is legal but rejected by llvm-mc since 86a60e7,
and this patch fixed that range issue.
@llvmbot llvmbot added the mc Machine (object) code label Sep 18, 2024
@llvmbot
Copy link
Member

llvmbot commented Sep 18, 2024

@llvm/pr-subscribers-mc

Author: Ben Shi (benshi001)

Changes

'rjmp .+4094' is legal but rejected by llvm-mc since 86a60e7, and this patch fixed that range issue.


Full diff: https://github.com/llvm/llvm-project/pull/109124.diff

2 Files Affected:

  • (modified) llvm/lib/Target/AVR/MCTargetDesc/AVRAsmBackend.cpp (+3-3)
  • (modified) llvm/test/MC/AVR/inst-rjmp.s (+22-18)
diff --git a/llvm/lib/Target/AVR/MCTargetDesc/AVRAsmBackend.cpp b/llvm/lib/Target/AVR/MCTargetDesc/AVRAsmBackend.cpp
index 388d58a82214d1..c0bc1276967bf0 100644
--- a/llvm/lib/Target/AVR/MCTargetDesc/AVRAsmBackend.cpp
+++ b/llvm/lib/Target/AVR/MCTargetDesc/AVRAsmBackend.cpp
@@ -88,15 +88,15 @@ static void adjustBranch(unsigned Size, const MCFixup &Fixup, uint64_t &Value,
 /// Adjusts the value of a relative branch target before fixup application.
 static void adjustRelativeBranch(unsigned Size, const MCFixup &Fixup,
                                  uint64_t &Value, MCContext *Ctx = nullptr) {
+  // Jumps are relative to the current instruction.
+  Value -= 2;
+
   // We have one extra bit of precision because the value is rightshifted by
   // one.
   signed_width(Size + 1, Value, std::string("branch target"), Fixup, Ctx);
 
   // Rightshifts the value by one.
   AVR::fixups::adjustBranchTarget(Value);
-
-  // Jumps are relative to the current instruction.
-  Value -= 1;
 }
 
 /// 22-bit absolute fixup.
diff --git a/llvm/test/MC/AVR/inst-rjmp.s b/llvm/test/MC/AVR/inst-rjmp.s
index 74d703593f6667..cf2a9d106f3d18 100644
--- a/llvm/test/MC/AVR/inst-rjmp.s
+++ b/llvm/test/MC/AVR/inst-rjmp.s
@@ -19,25 +19,28 @@ end:
 x:
   rjmp x
   .short 0xc00f
+  rjmp .+4094
 
-; CHECK: rjmp (.Ltmp0+2)+2  ; encoding: [A,0b1100AAAA]
-; CHECK-NEXT:               ;   fixup A - offset: 0, value: (.Ltmp0+2)+2, kind: fixup_13_pcrel
-; CHECK: rjmp (.Ltmp1-2)+2  ; encoding: [A,0b1100AAAA]
-; CHECK-NEXT:               ;   fixup A - offset: 0, value: (.Ltmp1-2)+2, kind: fixup_13_pcrel
-; CHECK: rjmp foo           ; encoding: [A,0b1100AAAA]
-; CHECK-NEXT:               ;   fixup A - offset: 0, value: foo, kind: fixup_13_pcrel
-; CHECK: rjmp (.Ltmp2+8)+2  ; encoding: [A,0b1100AAAA]
-; CHECK-NEXT:               ;   fixup A - offset: 0, value: (.Ltmp2+8)+2, kind: fixup_13_pcrel
-; CHECK: rjmp end           ; encoding: [A,0b1100AAAA]
-; CHECK-NEXT:               ;   fixup A - offset: 0, value: end, kind: fixup_13_pcrel
-; CHECK: rjmp (.Ltmp3+0)+2  ; encoding: [A,0b1100AAAA]
-; CHECK-NEXT:               ;   fixup A - offset: 0, value: (.Ltmp3+0)+2, kind: fixup_13_pcrel
-; CHECK: rjmp (.Ltmp4-4)+2  ; encoding: [A,0b1100AAAA]
-; CHECK-NEXT:               ;   fixup A - offset: 0, value: (.Ltmp4-4)+2, kind: fixup_13_pcrel
-; CHECK: rjmp (.Ltmp5-6)+2  ; encoding: [A,0b1100AAAA]
-; CHECK-NEXT:               ;   fixup A - offset: 0, value: (.Ltmp5-6)+2, kind: fixup_13_pcrel
-; CHECK: rjmp x             ; encoding: [A,0b1100AAAA]
-; CHECK-NEXT:               ;   fixup A - offset: 0, value: x, kind: fixup_13_pcrel
+; CHECK: rjmp (.Ltmp0+2)+2    ; encoding: [A,0b1100AAAA]
+; CHECK-NEXT:                 ;   fixup A - offset: 0, value: (.Ltmp0+2)+2, kind: fixup_13_pcrel
+; CHECK: rjmp (.Ltmp1-2)+2    ; encoding: [A,0b1100AAAA]
+; CHECK-NEXT:                 ;   fixup A - offset: 0, value: (.Ltmp1-2)+2, kind: fixup_13_pcrel
+; CHECK: rjmp foo             ; encoding: [A,0b1100AAAA]
+; CHECK-NEXT:                 ;   fixup A - offset: 0, value: foo, kind: fixup_13_pcrel
+; CHECK: rjmp (.Ltmp2+8)+2    ; encoding: [A,0b1100AAAA]
+; CHECK-NEXT:                 ;   fixup A - offset: 0, value: (.Ltmp2+8)+2, kind: fixup_13_pcrel
+; CHECK: rjmp end             ; encoding: [A,0b1100AAAA]
+; CHECK-NEXT:                 ;   fixup A - offset: 0, value: end, kind: fixup_13_pcrel
+; CHECK: rjmp (.Ltmp3+0)+2    ; encoding: [A,0b1100AAAA]
+; CHECK-NEXT:                 ;   fixup A - offset: 0, value: (.Ltmp3+0)+2, kind: fixup_13_pcrel
+; CHECK: rjmp (.Ltmp4-4)+2    ; encoding: [A,0b1100AAAA]
+; CHECK-NEXT:                 ;   fixup A - offset: 0, value: (.Ltmp4-4)+2, kind: fixup_13_pcrel
+; CHECK: rjmp (.Ltmp5-6)+2    ; encoding: [A,0b1100AAAA]
+; CHECK-NEXT:                 ;   fixup A - offset: 0, value: (.Ltmp5-6)+2, kind: fixup_13_pcrel
+; CHECK: rjmp x               ; encoding: [A,0b1100AAAA]
+; CHECK-NEXT:                 ;   fixup A - offset: 0, value: x, kind: fixup_13_pcrel
+; CHECK: rjmp (.Ltmp6+4094)+2 ; encoding: [A,0b1100AAAA]
+; CHECK-NEXT:                 ;   fixup A - offset: 0, value: (.Ltmp6+4094)+2, kind: fixup_13_pcrel
 
 ; INST-LABEL: <foo>:
 ; INST-NEXT: 01 c0      rjmp  .+2
@@ -54,3 +57,4 @@ x:
 ; INST-LABEL: <x>:
 ; INST-NEXT: ff cf      rjmp  .-2
 ; INST-NEXT: 0f c0      rjmp  .+30
+; INST-NEXT: ff c7      rjmp  .+4094

Copy link
Contributor

@Patryk27 Patryk27 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

Copy link
Contributor

@jacquesguan jacquesguan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@benshi001 benshi001 merged commit 8c3b94f into llvm:main Sep 20, 2024
10 checks passed
@benshi001 benshi001 deleted the avr-rjmp branch September 20, 2024 03:40
@Patryk27
Copy link
Contributor

@aykevl / @benshi001 could you create a backport to 19.1? (I think I can't assign milestones, so can't use the bot here)

@benshi001 benshi001 added this to the LLVM 19.X Release milestone Oct 14, 2024
@benshi001
Copy link
Member Author

@llvmbot Please back port this PR to release/19.x

@Patryk27
Copy link
Contributor

Hmm, seems like the backport didn't get created? 🤔

@Patryk27
Copy link
Contributor

/cherry-pick efcf52b

@llvmbot
Copy link
Member

llvmbot commented Oct 28, 2024

Failed to cherry-pick: efcf52b

https://github.com/llvm/llvm-project/actions/runs/11563217310

Please manually backport the fix and push it to your github fork. Once this is done, please create a pull request

@Patryk27
Copy link
Contributor

Ah!

@Patryk27
Copy link
Contributor

/cherry-pick 8c3b94f

@llvmbot
Copy link
Member

llvmbot commented Oct 28, 2024

/pull-request #113969

tru pushed a commit to llvmbot/llvm-project that referenced this pull request Oct 29, 2024
'rjmp .+4094' is legal but rejected by llvm-mc since
86a60e7, and this patch fixed that
range issue.

(cherry picked from commit 8c3b94f)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mc Machine (object) code
Projects
Development

Successfully merging this pull request may close these issues.

4 participants