This is an archive of the discontinued LLVM Phabricator instance.

[Mips] Add glue between CopyFromReg, CopyToReg and RDHWR nodes for TLS
ClosedPublic

Authored by jrtc27 on Oct 17 2021, 10:08 AM.

Details

Summary

The MIPS ABI requires the thread pointer be accessed via rdhwr $3, $r29.
This is currently represented by (CopyToReg $3, (RDHWR $29)) followed by
a (CopyFromReg $3). However, there is no glue between these, meaning
scheduling can break those apart. In particular, PR51691 is a report
where PseudoSELECT_I was moved to between the CopyToReg and CopyFromReg,
and since its expansion uses branches, it split the def and use of the
physical register between two basic blocks, resulting in the def being
eliminated and the use having no def. It also seems possible that a
similar situation could arise splitting up the CopyToReg from the RDHWR,
causing the RDHWR to use a destination register other than $3, violating
the ABI requirement.

Thus, add glue between all three nodes to ensure they aren't split up
during instruction selection. No regression test is added since any test
would be implictly relying on specific scheduling behaviour, so whilst
it might be testing that glue is preventing reordering today, changes to
scheduling behaviour could result in the test no longer being able to
catch a regression here, as the reordering might no longer happen for
other unrelated reasons.

Fixes PR51691.

Diff Detail

Event Timeline

jrtc27 created this revision.Oct 17 2021, 10:08 AM
jrtc27 requested review of this revision.Oct 17 2021, 10:08 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 17 2021, 10:08 AM
atanasyan accepted this revision.Oct 18 2021, 12:50 AM

LGTM. Thanks for the fix.

This revision is now accepted and ready to land.Oct 18 2021, 12:50 AM
dim accepted this revision.Oct 18 2021, 5:49 AM

This worked OK for building both 32-bit and 64-bit mips worlds on FreeBSD. Thanks!

Is this worth backporting to 13, given a change in 13 is what exposed the underlying bug for FreeBSD? We can carry patches for FreeBSD but that won't help other people (or people using non-FreeBSD-originated compilers to compile FreeBSD, i.e. using a system LLVM on macOS or Linux to cross-compile it).

This revision was landed with ongoing or failed builds.Oct 18 2021, 7:11 AM
This revision was automatically updated to reflect the committed changes.

Ah I see the PR is already blocking release-13.0.1 (and release-13.0.0, but that ship has sailed).