This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Add CostModel GEP tests
ClosedPublic

Authored by luismarques on Apr 26 2019, 5:44 AM.

Details

Summary

These tests revolve around checking that the implied offsets fit the simm12 immediates. When they do, the offsets can be folded into the loads/stores, and therefore they should have zero cost.
The patch adds vector tests, despite the RISC-V backend currently not targetting any vector units. That was a borderline decision. Since we can compile IR vector instructions to scalar native ones, I felt including the vector tests made sense. This way we also have the infrastructure already in place for when we decide to add support for vector extensions.

Diff Detail

Repository
rL LLVM

Event Timeline

luismarques created this revision.Apr 26 2019, 5:44 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 26 2019, 5:44 AM
asb requested changes to this revision.May 23 2019, 3:58 AM

I think it would make sense to combine the RV32I and RV64I check lines into a shared RVI check-prefix, as I don't think we're expecting different cost model answers for RV32 vs RV64?

This revision now requires changes to proceed.May 23 2019, 3:58 AM
In D61185#1513477, @asb wrote:

I think it would make sense to combine the RV32I and RV64I check lines into a shared RVI check-prefix, as I don't think we're expecting different cost model answers for RV32 vs RV64?

That sounds sensible. I did a quick review, to see if there might have been a plausible reason for the split, and I couldn't find any. Will change.

luismarques retitled this revision from [RISCV] Add CostModel tests to [RISCV] Add CostModel GEP tests.

Removed the RV64I check prefixes and renamed the RV32I prefix to RVI.

asb added inline comments.May 24 2019, 7:24 AM
test/Analysis/CostModel/RISCV/gep.ll
2 ↗(On Diff #201189)

You want a -mtriple=riscv64 RUN line here, but with the same RVI check-prefix

Now with two check lines, checking against a common prefix.

luismarques marked an inline comment as done.May 29 2019, 8:32 AM
asb accepted this revision.May 30 2019, 2:17 AM

LGTM with minor nit

test/Analysis/CostModel/RISCV/gep.ll
18 ↗(On Diff #201244)

Nit: I think these empty comment lines are unwanted and an artifact of when there were two different check prefixes

This revision is now accepted and ready to land.May 30 2019, 2:17 AM
This revision was automatically updated to reflect the committed changes.
luismarques marked 2 inline comments as done.Jun 6 2019, 2:46 AM
luismarques added inline comments.
test/Analysis/CostModel/RISCV/gep.ll
18 ↗(On Diff #201244)

As discussed with @asb in person, and for future reference, update_analyze_test_checks.py does add the empty comment lines, while update_llc_test_checks.py does not.