This is an archive of the discontinued LLVM Phabricator instance.

[ARM] Add v8m.base pattern for add negative imm
ClosedPublic

Authored by samparker on Feb 8 2019, 2:53 AM.

Details

Summary

The v8m.base ISA contains movw, which can operate on an unsigned 16-bit value. Add the pattern that converts an add with a negative value, that could fit into 16-bits when negated, into a sub with that positive value.

Diff Detail

Repository
rL LLVM

Event Timeline

samparker created this revision.Feb 8 2019, 2:53 AM
fhahn added inline comments.Feb 8 2019, 4:19 AM
test/CodeGen/ARM/sub.ll
10 ↗(On Diff #185935)

Why is this change and the similar ones needed?

67 ↗(On Diff #185935)

Check-label

73 ↗(On Diff #185935)

Check-label

Also it might be worth expanding the check for v6m a bit

samparker marked an inline comment as done.Feb 8 2019, 4:42 AM
samparker added inline comments.
test/CodeGen/ARM/sub.ll
10 ↗(On Diff #185935)

because we're testing arm and thumb code, where we maybe using the destructive form.

samparker updated this revision to Diff 185948.Feb 8 2019, 4:51 AM

Updated tests.

dmgreen added inline comments.Feb 8 2019, 5:14 AM
lib/Target/ARM/ARMInstrThumb.td
1326 ↗(On Diff #185948)

Should this be t2MOVi16?

samparker marked an inline comment as done.Feb 8 2019, 5:29 AM
samparker added inline comments.
lib/Target/ARM/ARMInstrThumb.td
1326 ↗(On Diff #185948)

yup, cheers!

Before I put this in ISelDAGToDAG, is there a way to reference machine instructions from across different tablegen files..?

As far as understand, ARMInstrThumb.td is just #included in the other tablegen file. I guess the ordering might be important for them. Would it work better if it's defined in ARMInstrThumb2.td?

Well, I'll still have the problem that tSUBrr is defined in the other file =/

samparker updated this revision to Diff 185968.Feb 8 2019, 7:26 AM

I've opted for putting the pattern in ARMInstrInfo once everything has been included. Also added encodings to the test to ensure we're now using the correct movw.

It looks like there are already a couple of patterns in ARMInstrThumb2.td for thumb1 instructions. I think with a comment it would be fine to put it in there. HasV8MBaseline tends to cross Thumb1/Thumb2 already.

samparker updated this revision to Diff 186215.Feb 11 2019, 3:14 AM

Cheers Dave!

I've moved the pattern to where the accompanying T2 patterns are defined.

dmgreen accepted this revision.Feb 11 2019, 3:20 AM

LGTM

This revision is now accepted and ready to land.Feb 11 2019, 3:20 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptFeb 11 2019, 3:35 AM