This is an archive of the discontinued LLVM Phabricator instance.

[GlobalISel] Implement widenScalar for carry-in add/sub
ClosedPublic

Authored by porglezomp on Jan 24 2021, 6:05 PM.

Details

Summary

These are widened to a wider UADDE/USUBE, with the overflow value
unused, and with the same synthesis of a new overflow value as for the
O operations.

Diff Detail

Event Timeline

porglezomp created this revision.Jan 24 2021, 6:05 PM
porglezomp requested review of this revision.Jan 24 2021, 6:05 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 24 2021, 6:05 PM

Update tests to match run-line changes

arsenm added a subscriber: arsenm.Jan 25 2021, 11:08 AM
arsenm added inline comments.
llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
1768

Should just use a plain Register here, MachineOperand values are generally bad

1817–1818

This is .getReg(0)

Change MachineOperand variable to Register, use getReg(0) on MachineInstrBuilder

Add unit-tests in LegalizerHelperTest

paquette added inline comments.Jan 26 2021, 9:59 AM
llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
1769

Can you add an unreachable case to make sure nobody passes an unexpected instruction here?

llvm/test/CodeGen/AArch64/GlobalISel/legalize-sadde.mir
5

Can these testcases have slightly more descriptive names?

32

Nit: we can give virtual registers names, and it usually makes it way easier to read + understand testcases (IMO).

Since the new testcases here are all pretty similar, do you think you could add names to the virtual registers?

Add unreachable case, give vregs names in tests

This revision is now accepted and ready to land.Jan 28 2021, 9:45 AM
This revision was landed with ongoing or failed builds.Jan 28 2021, 2:06 PM
This revision was automatically updated to reflect the committed changes.