This is an archive of the discontinued LLVM Phabricator instance.

Support inline asm with multiple 64bit output in 32bit GPR
ClosedPublic

Authored by thopre on Apr 9 2018, 5:37 AM.

Details

Summary

Extend fix for PR34170 to support inline assembly with multiple output operands that do not naturally go in the register class it is constrained to (eg. double in a 32-bit GPR as in the PR).

Diff Detail

Event Timeline

thopre created this revision.Apr 9 2018, 5:37 AM
thopre updated this revision to Diff 156999.Jul 24 2018, 4:09 AM

Rename testcase and test both soft and hard float ABI

thopre updated this revision to Diff 157791.Jul 27 2018, 3:27 PM

Rebase on latest version of previous patch in stack

thopre updated this revision to Diff 157980.Jul 30 2018, 9:09 AM

Rebase and rename testcase for consistency

thopre added a comment.Aug 3 2018, 7:41 AM

Ping? This fix the bug reported in https://reviews.llvm.org/rL337903

tra added a subscriber: tra.Aug 3 2018, 11:13 AM

This fix the bug reported in https://reviews.llvm.org/rL337903

Do we have test coverage for that issue reported in that email? If not, please add a testcase.

lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
7683

isSized()? How can you end up with an unsized value here?

7703

This part of the patch is just whitespace changes?

test/CodeGen/ARM/inlineasm-operand-implicit-cast.ll
175 ↗(On Diff #157980)

Please add a test for multiple return values with matching inputs.

thopre added inline comments.Aug 7 2018, 6:41 AM
lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
7683

It was in the original code so I kept it. I've now changed it to an assert and can confirm the testsuite shows no regression.

7703

The comment yes, the rest is s/ResultType/ResultVT

thopre updated this revision to Diff 159498.Aug 7 2018, 6:43 AM
  • Change test for isSized() into assert
  • Add test for multiple return values with matching input
  • Rewrite hardfloat tests to return a struct to make register used predictable and not dependent on register allocator
This revision is now accepted and ready to land.Aug 7 2018, 11:23 AM
This revision was automatically updated to reflect the committed changes.