- User Since
- Aug 6 2013, 5:31 AM (223 w, 6 d)
Rebased and updated to implement getFrameIndexReference.
As mentioned on the mailing list I don't have time to fix up and audit the test case changes. If anyone wants to see BUNDLE with hasSideEffects=0, please do commandeer the revision.
Updated to add a TableGen test for the generation of MatchRegisterName and MatchRegisterAltName when AllowDuplicateRegisterNames is set.
It would have been much cleaner if it worked as @efriedma suggests and -mfloat-abi was only concerned with the ABI (as its name would suggest), but sadly the -mfloat-abi=soft option seems to be defined in GCC to control more than just the ABI.
Thu, Nov 16
I should add a clarifying comment. These are codegen-only constructs that as far as I can see are required for frameindex lowering, in order to represent a frameindex+offset pair.
This is the correct string, as used and tested in the yet-to-be merged patches. I forgot to move this fix from the relevant RV64I patch. Thanks for noticing this.
Consider this a WIP update. This is not yet ready for merging, but could still benefit from feedback.
Thanks, and do feel free to directly commit build fixes like this if you have commit access without pre-commit review.
Wed, Nov 15
Updated to address review comments. I've added some extra test coverage that demonstrates that argument lowering happens the same once registers are exhausted, as well as more coverage around varargs. Also updated to properly handle the "aligned register pair" rule for variadic arguments, and added tests for this.
Tue, Nov 14
Thanks Shiva. A whole bunch of inline comments are below. Thanks to Simon for spotting the missing -triple in the tests, I was struggling to see why I couldn't get the tests to pass.
Updated patch to address review comments.
Mon, Nov 13
Thanks Reid, updated the patch with that change. I'll aim to commit tomorrow.
Rebase on latest LLVM and remove redundant return in eliminateFrameIndex.
Updated patch to address review comments (thanks!).
Sat, Nov 11
Fri, Nov 10
Small fix in rv32f-invalid.s.
Adds a bare-select.ll test case, ensuring all code paths in lowerSELECT are tested.
Thu, Nov 9
Looks good to me, thanks! Please go ahead and commit.
Address @mgrang's review comments (thanks!). I also moved the changes to eliminateFrameIndex to the previous patch, as it makes sense to just do it right from the start.
Thanks Mandeep, updated the patch. I've moved a bit of logic from the successor patch (D39849) to this one, as I think it makes more sense to use the correct register in eliminateFrameIndex from the start, rather than adding the extra logic in the next patch.
Wed, Nov 8
Updated to address review comments from @reames. Tests are now generated using utils/update_llc_test_checks.py.
This minor revision of the patch switches to lowering ISD::SELECT. This is motivated by future support for the floating point instruction set extensions and the coarse-grained nature of setOperationAction. See the updated patch description and new code comments for more discussion, but essentially we need to ensure that a RISC-V compare+branch instruction is generated whenever the comparison is between two XLEN integers.
Thanks for helping to unblock this Philip. I was holding off on committing as Chandler was hoping to corral another backend maintainer into giving a second opinion. Unfortunately it looks like that person didn't have time, so I'm going ahead and committing.
This looks good to me. Tests for invalid operands etc as Florian suggests are definitely worth having. ARM, Mips and RISCV tend to add this in test files prefixed or suffixed with invalid, e.g. add-invalid.s or sve-invalid.s. AArch64 seems to use a diagnostics suffix.
Tue, Oct 31
I'm not an LLD expert, but the RISC-V bits seem correct as far as I can see. I've just added a couple of minor comments.
Mon, Oct 23
The change to Lanai.td should not have been included. Refreshing patch.
Oct 20 2017
Split out the immediate materialisation tests to a separate file and add a TODO for immediates with lo12=0.
Oct 19 2017
I accidentally ttached an older version of the patch before, which had an unwanted whitespace change.
Updated to split constant materialization to a preceding patch (D39101) and global addresses to a succeeding patch (to be posted shortly).
Address review comments from Philip (thanks!).