This is an archive of the discontinued LLVM Phabricator instance.

[ARC] Add additional mov immediate instruction formats with a fix for u6 decoding
ClosedPublic

Authored by thomasjohns on Jul 29 2021, 10:32 AM.

Details

Summary

This adds the mov_f_ru6 and mov_cc_f_ru6 instruction formats. Adding the f flag to these instructions revealed a bug in the u6 decoding (details to be explained in context below). This u6 decoding bug is fixed here.

Many new decoder test cases have also been added, exercising the changes mentioned above in addition to further exercising predicated binary instructions (the support for these was added here https://reviews.llvm.org/rG1cda1e618648d463f89eb5cea8c9849bc0074b24, but that patch only tested predicated rsub).

Diff Detail

Event Timeline

thomasjohns created this revision.Jul 29 2021, 10:32 AM
thomasjohns requested review of this revision.Jul 29 2021, 10:32 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 29 2021, 10:32 AM
thomasjohns added inline comments.Jul 29 2021, 10:40 AM
llvm/lib/Target/ARC/ARCInstrFormats.td
289

This change was really just a rename from F32_SOP_CC_RU6 to F32_DOP_CC_RU6. The instruction is really a "dual-operand" instruction, and was just named incorrectly earlier.

The full diff shows up rather than the rename because the definition was moved to be adjacent to other dual operand instructions.

367

This field was unused.

llvm/lib/Target/ARC/Disassembler/ARCDisassembler.cpp
307

This API was misinterpreted earlier to be fieldFromInstruction(..., startBit, endBit) instead of fieldFromInstruction(..., startBit, numBits) (correct). This didn't materialize earlier because all of the associated tests had 0'd out high order bits. So overreading the bits for the u6 instruction wasn't noticed because it didn't change the u6 value.

Once the f flag was added, it caused the u6 decoding here to overflow, because the f flag bit was 11 bits away from the u6 start bit.

marksl added inline comments.Jul 29 2021, 11:54 AM
llvm/test/MC/Disassembler/ARC/alu.txt
219

You should probably check something other than R0 to verify the field is defined correctly. R0 will just encode zeros which may not show up an error.

thomasjohns added inline comments.Jul 29 2021, 12:13 PM
llvm/test/MC/Disassembler/ARC/alu.txt
219

Okay thanks, that's a good idea. I will update the patch with more varied register numbers.

Make register numbers and immediate values more varied in decoder tests.

marksl accepted this revision.Jul 29 2021, 1:53 PM

Looks good to me

This revision is now accepted and ready to land.Jul 29 2021, 1:53 PM
This revision was landed with ongoing or failed builds.Jul 29 2021, 4:46 PM
This revision was automatically updated to reflect the committed changes.