Skip to content

release/19.x: [AVR][MC] Fix incorrect range of relative jumps (#109124) #113969

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
Oct 29, 2024

Conversation

llvmbot
Copy link
Member

@llvmbot llvmbot commented Oct 28, 2024

Backport 8c3b94f

Requested by: @Patryk27

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

llvmbot commented Oct 28, 2024

@jacquesguan What do you think about merging this PR to the release branch?

@llvmbot llvmbot requested a review from jacquesguan October 28, 2024 22:02
@llvmbot llvmbot added the mc Machine (object) code label Oct 28, 2024
@llvmbot
Copy link
Member Author

llvmbot commented Oct 28, 2024

@llvm/pr-subscribers-mc

Author: None (llvmbot)

Changes

Backport 8c3b94f

Requested by: @Patryk27


Full diff: https://github.com/llvm/llvm-project/pull/113969.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 cc843a58b55d2c..2d7aa401feacf0 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

@Patryk27
Copy link
Contributor

A little context - I'd like to get it merged to 19.x, so that I can pull it to rustc - it fixes a minor codegen issue within the AVR backend, which causes some firmwares not to get compiled, crashing LLVM.

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
'rjmp .+4094' is legal but rejected by llvm-mc since
86a60e7, and this patch fixed that
range issue.

(cherry picked from commit 8c3b94f)
@tru tru merged commit 936710a into llvm:release/19.x Oct 29, 2024
7 of 9 checks passed
Copy link

@Patryk27 (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR.

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.

None yet

5 participants