This is an archive of the discontinued LLVM Phabricator instance.

Add numeric extend, trunctate to mips fast-isel
ClosedPublic

Authored by rkotler on Sep 8 2014, 12:38 PM.

Details

Diff Detail

Event Timeline

rkotler updated this revision to Diff 13414.Sep 8 2014, 12:38 PM
rkotler retitled this revision from to Add numeric extend, trunctate to mips fast-isel.
rkotler updated this object.
rkotler edited the test plan for this revision. (Show Details)
rkotler added a reviewer: dsanders.
rkotler added a subscriber: mcrosier.
rkotler updated this revision to Diff 13851.Sep 18 2014, 3:12 PM
  • Merge branch 'master' into reed-bug-04
  • in progress
  • Merge branch 'fast-isel-16' into reed-bug-04
  • Merge branch 'master' into reed-bug-04
  • in progress
  • in progress
  • in progress
  • Merge branch 'master' into reed-bug-04
  • test update
  • test update
  • test update
  • test update
  • format
  • format
  1. Updating D5251: Add numeric extend, trunctate to mips fast-isel #
  2. Enter a brief description of the changes included in this update.
  3. The first line is used as subject, next lines as comment. #
  4. If you intended to create a new revision, use:
  5. $ arc diff --create

Update patch to handler Mips32/r1

rkotler updated this revision to Diff 13855.Sep 18 2014, 3:59 PM
  • make sure test for mips32r1
  1. Updating D5251: Add numeric extend, trunctate to mips fast-isel #
  2. Enter a brief description of the changes included in this update.
  3. The first line is used as subject, next lines as comment. #
  4. If you intended to create a new revision, use:
  5. $ arc diff --create

Add an extra test for mips32r1

dsanders edited edge metadata.Sep 25 2014, 1:20 PM

It will LGTM with a nit, the CHECK-LABEL change, and mcrosiers review comments from the original review (D4827) handled.

There's one additional change from the original review that I'd like you to make but it's no longer a hard requirement.

Add numeric extend, trunctate to mips fast-isel

Reactivates D4827

Will be updating patch to fix issues previously reported.

Just a quick reminder that 'arc commit' will use this text as-is in the commit message. You'll need to edit the revision to drop the last two lines of the summary or commit with svn.

On a similar note, you seem to have arcanist instructions in the description of your two updates.

lib/Target/Mips/MipsFastISel.cpp
257–267

Per my original comments in D4827, I'd prefer this to be calculated rather than special cased for each type. However, you've handled the most important point of those comments (MIPS32r1 support) so while I'd appreciate it using 'ShiftAmt = GPRSize - SrcVT.getSizeInBits()' and a separate if-statement. I'm not going to make that a hard requirement.

Likewise for the ZExt cases.

295–296

Coding standard nit: return after else.

test/CodeGen/Mips/Fast-ISel/loadstoreconv.ll
31

Copy/pasted from D4827:
Use CHECK-LABEL so that FileCheck divides the file into independent sections. Likewise for the other function labels below.

rkotler updated this revision to Diff 14198.Sep 29 2014, 6:48 PM
rkotler edited edge metadata.
  • make suggested changes. still need to run test-suite before commit
rkotler updated this revision to Diff 14227.Sep 30 2014, 8:21 AM
  • clang format
  1. Updating D5251: Add numeric extend, trunctate to mips fast-isel #

reformat MipsFastISel.cpp using clang format

rkotler updated this object.Sep 30 2014, 9:38 AM
rkotler closed this revision.Sep 30 2014, 9:40 AM
rkotler updated this revision to Diff 14232.

Closed by commit rL218681 (authored by @rkotler).