This is an archive of the discontinued LLVM Phabricator instance.

[RISCV][NFC] Regenerate RISCV CodeGen tests
ClosedPublic

Authored by mundaym on Dec 8 2020, 6:05 AM.

Details

Summary

Regenerated using:

./llvm/utils/update_llc_test_checks.py -u llvm/test/CodeGen/RISCV/*.ll

This has added comments to spill-related instructions and added @plt to
some symbols.

Diff Detail

Event Timeline

mundaym created this revision.Dec 8 2020, 6:05 AM
mundaym requested review of this revision.Dec 8 2020, 6:05 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 8 2020, 6:05 AM
lenary added a comment.Dec 8 2020, 6:48 AM

The spills look right to me. The question we're not sure of is the appearance of the @plt suffixes on all the external calls.

jrtc27 added a comment.Dec 8 2020, 7:00 AM

The spills look right to me. The question we're not sure of is the appearance of the @plt suffixes on all the external calls.

RISC-V's calls are a mess. Without @plt you get an R_RISCV_CALL, and with @plt you get an R_RISCV_CALL_PLT. In LLD both are treated identically. In binutils, the latter always works (it'll get a PLT stub if needed, but not otherwise), whereas the former can cause weird things to happen if you need a PLT stub (and it's dependent on what other relocations for the symbol exist IIRC). Basically, R_RISCV_CALL should never have existed and R_RISCV_CALL_PLT should have been R_RISCV_CALL, but if you want to support linking with binutils then you need the @plt suffix unless you know the symbol will always be local.

See https://github.com/riscv/riscv-elf-psabi-doc/issues/98, https://sourceware.org/bugzilla/show_bug.cgi?id=24685 and D63076.

@MaskRay am I missing anything?

lenary added a comment.Dec 8 2020, 1:05 PM

@jrtc27 it sounds like adding @plt into all the tests won't break anything, and will catch if future changes default to non-plt calls, so this change is an improvement?

jrtc27 added a comment.Dec 8 2020, 1:06 PM

@jrtc27 it sounds like adding @plt into all the tests won't break anything, and will catch if future changes default to non-plt calls, so this change is an improvement?

Yes I believe so.

lenary accepted this revision.Dec 8 2020, 2:19 PM

LGTM

This revision is now accepted and ready to land.Dec 8 2020, 2:19 PM
This revision was landed with ongoing or failed builds.Dec 9 2020, 11:43 AM
This revision was automatically updated to reflect the committed changes.
llvm/test/CodeGen/RISCV/atomic-load-store.ll