This is an archive of the discontinued LLVM Phabricator instance.

[ARM64] Split tbz/tbnz into W/X register variant
ClosedPublic

Authored by bsmith on May 15 2014, 2:50 AM.

Details

Reviewers
t.p.northover
Summary

This patch splits tbz/tbnz into 32-bit/64-bit register variants, this allows us to properly emit diagnostics for out of range immediates and also removes some manual handling of these instructions.

As with my previous patches, there is no testcase for this as it is part of the merge of MC/AArch64/basic-a64-diagnostics.s.

Diff Detail

Event Timeline

bsmith updated this revision to Diff 9427.May 15 2014, 2:50 AM
bsmith retitled this revision from to [ARM64] Split tbz/tbnz into W/X register variant.
bsmith updated this object.
bsmith edited the test plan for this revision. (Show Details)
bsmith added a reviewer: t.p.northover.
bsmith set the repository for this revision to rL LLVM.
bsmith added a subscriber: Unknown Object (MLST).
t.p.northover edited edge metadata.May 15 2014, 4:20 AM

Hi Bradley,

I've got a suggestion on this one:

lib/Target/ARM64/ARM64InstrFormats.td
1097–1102

This has the quirk that if someone assembles "tbz x3, #0" that's what'll be printed, doesn't it?

I think we can improve on this (both AArch64 and ARM64's solution) if we add a new RegisterOperand: "GPR32as64" or something. It'll check that the actually parsed operand is a GPR64, but add the corresponding GPR32 to the instruction in its RenderMethod.

With this, we *can* write the InstAlias, I believe (remember to set the Emit bit to 0!). It'll probably be a little bit more code, but worth it.

bsmith updated this revision to Diff 9446.May 15 2014, 9:33 AM
bsmith edited edge metadata.

Updated patch to properly emit w-registers when using an immediate < 32.

Hi Bradley,

Nearly there, but I think there's still one problem:

lib/Target/ARM64/ARM64InstrFormats.td
1068–1070

The ImmLeaf predicate here looks wrong.

If you want to match "(ARM64tbz GPR64:$Rn, 0)" it needs to go via a separate Pat using EXTRACT_SUBREG. Otherwise you'll create an instruction like "TBZX %x0, 0", which may print plausibly but will almost certainly be encoded incorrectly.

test/CodeGen/AArch64/analyze-branch.ll
150 ↗(On Diff #9446)

And this is the test-level manifestation of the bug.

bsmith updated this revision to Diff 9466.May 16 2014, 4:15 AM

Fixup matching of X-reg variant with imm<31. This was incorrectly being selected as tbz x0,#0, rather than tbz w0,#0.

t.p.northover accepted this revision.May 16 2014, 4:22 AM
t.p.northover edited edge metadata.

Hi Bradley,

This looks good to me now. Thanks for working on it for so long!

Tim.

This revision is now accepted and ready to land.May 16 2014, 4:22 AM
bsmith closed this revision.May 19 2014, 9:05 AM