This is an archive of the discontinued LLVM Phabricator instance.

Differential D6842

[ARM] Fix BXJ is undefined in AArch32.
AbandonedPublic

Authored by rengolin on Jan 5 2015, 7:06 AM.

Details

Reviewers
rengolin
• chatur01
olista01
Summary

BXJ was incorrectly said to be unsupported in ARMv8-A. It is not supported in
the A64 instruction set, but it is supported in the T32 and A32 instruction
sets, because it's listed as an instruction in the ARM ARM section F7.1.28.

This was found by MCHammer.

Diff Detail

Event Timeline

• chatur01 updated this revision to Diff 17797.Jan 5 2015, 7:06 AM
• chatur01 retitled this revision from to [ARM] Fix BXJ is undefined in AArch32..
• chatur01 updated this object.
• chatur01 edited the test plan for this revision. (Show Details)
• chatur01 set the repository for this revision to rL LLVM.
• chatur01 added a subscriber: Unknown Object (MLST).
Herald added a subscriber: aemerson. · View Herald TranscriptJan 5 2015, 7:06 AM
• chatur01 added a comment.Jan 12 2015, 1:27 AM

Ping. :)

olista01 requested changes to this revision.Jan 12 2015, 2:34 AM
olista01 added a reviewer: olista01.
olista01 added a subscriber: olista01.

Up to v7[AR], BXJ is unpredictable for r13 and r15, but v8A makes it well-defined for r13. This should be accepted for v8A thumb:

$ llvm-mc -triple thumbv8a--none-eabi -assemble <<< "bxj r13"
        .text
<stdin>:1:1: error: instruction requires: arm-mode
bxj r13
^
This revision now requires changes to proceed.Jan 12 2015, 2:34 AM
• chatur01 updated this revision to Diff 23151.EditedApr 2 2015, 8:05 AM
• chatur01 edited edge metadata.

Track the change from SP being unpredictable as an operand to BXJ in architecture pre v8-A to now being predictable.

I wasn't sure how best to approach the unpredictability change in v8-A.
The approach here was to change rGPR to GPRnopc, so that BXJ could now match SP as an operand, and then to add a new validate case in ARMAsmParser that check what the target architecture revision is. If it's v8-A, then BXJ SP is fine, otherwise give a diagnostic.
Is there a better way in tblgen to overload the register set an instruction can take based on the target architecture?

Herald added a subscriber: rengolin. · View Herald TranscriptApr 2 2015, 8:05 AM
• chatur01 added a comment.Apr 9 2015, 4:54 AM

Ping.

rengolin accepted this revision.Apr 15 2015, 5:39 AM
rengolin added a reviewer: rengolin.

LGTM, Thanks!

• chatur01 added a comment.Apr 15 2015, 10:34 AM

Thanks Renato! Landed r235024.

rengolin accepted this revision.May 8 2015, 4:38 AM
rengolin commandeered this revision.May 8 2015, 5:04 AM
rengolin accepted this revision.
rengolin abandoned this revision.
rengolin edited reviewers, added: • chatur01; removed: rengolin.
rengolin added a reviewer: rengolin.

Revision Contents

PathSize
lib/
Target/
ARM/
2 lines
test/
MC/
ARM/
2 lines

Diff 17797

lib/Target/ARM/ARMInstrThumb2.td

Show First 20 Lines • Show All 91 Lines • ▼ Show 20 Lines
let Inst{3-0} = mask; let Inst{3-0} = mask;
let DecoderMethod = "DecodeIT"; let DecoderMethod = "DecodeIT";
} }
// Branch and Exchange Jazelle -- for disassembly only // Branch and Exchange Jazelle -- for disassembly only
// Rm = Inst{19-16} // Rm = Inst{19-16}
def t2BXJ : T2I<(outs), (ins rGPR:$func), NoItinerary, "bxj", "\t$func", []>, def t2BXJ : T2I<(outs), (ins rGPR:$func), NoItinerary, "bxj", "\t$func", []>,
Sched<[WriteBr]>, Requires<[IsThumb2, IsNotMClass, PreV8]> { Sched<[WriteBr]>, Requires<[IsThumb2, IsNotMClass]> {
bits<4> func; bits<4> func;
let Inst{31-27} = 0b11110; let Inst{31-27} = 0b11110;
let Inst{26} = 0; let Inst{26} = 0;
let Inst{25-20} = 0b111100; let Inst{25-20} = 0b111100;
let Inst{19-16} = func; let Inst{19-16} = func;
let Inst{15-0} = 0b1000111100000000; let Inst{15-0} = 0b1000111100000000;
} }
▲ Show 20 Lines • Show All 91 Lines • Show Last 20 Lines

test/MC/ARM/thumb2-bxj.s

@ RUN: llvm-mc -triple=thumbv6t2--none-eabi -show-encoding < %s | FileCheck %s @ RUN: llvm-mc -triple=thumbv6t2--none-eabi -show-encoding < %s | FileCheck %s
@ RUN: llvm-mc -triple=thumbv7a--none-eabi -show-encoding < %s | FileCheck %s @ RUN: llvm-mc -triple=thumbv7a--none-eabi -show-encoding < %s | FileCheck %s
@ RUN: llvm-mc -triple=thumbv7r--none-eabi -show-encoding < %s | FileCheck %s @ RUN: llvm-mc -triple=thumbv7r--none-eabi -show-encoding < %s | FileCheck %s
@ RUN: llvm-mc -triple=thumbv8a--none-eabi -show-encoding < %s | FileCheck %s
@ RUN: not llvm-mc -triple=thumbv7m--none-eabi -show-encoding < %s 2>&1 | FileCheck %s --check-prefix=UNDEF @ RUN: not llvm-mc -triple=thumbv7m--none-eabi -show-encoding < %s 2>&1 | FileCheck %s --check-prefix=UNDEF
@ RUN: not llvm-mc -triple=thumbv8a--none-eabi -show-encoding < %s 2>&1 | FileCheck %s --check-prefix=UNDEF
bxj r2 bxj r2
@ CHECK: bxj r2 @ encoding: [0xc2,0xf3,0x00,0x8f] @ CHECK: bxj r2 @ encoding: [0xc2,0xf3,0x00,0x8f]
@ UNDEF: error: instruction requires: arm-mode @ UNDEF: error: instruction requires: arm-mode